lkml.org 
[lkml]   [2020]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 3/5] tracing: Update synth command errors
On Tue, 8 Dec 2020 12:53:40 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 08 Dec 2020 11:34:41 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
>
> > Unfortunately, you're correct, if you have a script that creates a
> > synthetic event without semicolons, this patchset will break it, as I
> > myself found out and fixed in patch 4 ([PATCH v3 4/5] selftests/ftrace:
> > Add synthetic event field separators) [4].
> >
> > So whereas before this would work, even though it shouldn't have in the
> > first place:
> >
> > # echo 'wakeup_latency u64 lat pid_t pid char comm[16]' >
> > synthetic_events
> >
> > it now has to be:
> >
> > # echo 'wakeup_latency u64 lat; pid_t pid; char comm[16]' >
> > synthetic_events
> >
> > So yeah, this patchset fixes a set of parsing bugs for things that
> > shouldn't have been accepted as valid, but shouldn't break things that
> > are obviously valid.
> >
> > If it's too late to fix them, though, I guess we'll just have to live
> > with them, or some other option?
>
>
> I would suggest allowing the old interface work (with no new features, for
> backward compatibility), but new things like "char comm[16]" we require
> semicolons.
>
> One method to do this is to add to the start of reading the string, and
> checking if it has semicolons. If it does not, we create a new string with
> them, but make sure that the string does not include new changes.
>
> strncpy_from_user(buffer, user_buff, sizeof(buffer));
>
> if (!strstr(buffer, ";")) {
> if (!audit_old_buffer(buffer))
> goto error;
> insert_colons(buffer);
> }
>
>
> That is, if the buffer does not have semicolons, then check if it is a
> valid "old format", and if not, we error out. Otherwise, we insert the
> colons into the buffer, and process that as if the user put in colons:
>
> That is:
>
> echo 'wakeup_latency u64 lat pid_t pid' > synthetic_events
>
> would change the buffer to:
>
> "wakeup_latency u64 lat; pid_t pid;"
>
> And then put it through the normal processing. I think its OK that if the
> user were to cat out the synthetic events, it would see the semicolons even
> if it did not add them. As I don't think that will break userspace.
>
> Does that make sense?

This makes sense. Anyway, what I considered were
- synthetic_events interface doesn't provide syntax error reports
- synthetic_events interface is not self-reproducive*.

*) I meant

$ cat synthetic_events > saved_events
$ cat saved_events > synthetic_events

should work. But this does *NOT* mean

$ cat user-input > synthetic_events
$ cat synthetic_events > saved_events
$ diff user-input saved_events # no diff

So input and output can be different, but the output can be input again.

Thank you,

--
Masami Hiramatsu <mhiramat@kernel.org>

\
 
 \ /
  Last update: 2020-12-09 14:53    [W:0.079 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site