Re: [RFC PATCH 4/7] tracing/probe: Check event/group naming rule at parsing

From: Masami Hiramatsu
Date: Wed Mar 13 2019 - 11:04:10 EST


On Wed, 13 Mar 2019 09:23:55 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> On Wed, 13 Mar 2019 21:28:12 +0900
> Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
>
> > Check event and group naming rule at parsing it instead
> > of allocating probes.
> >
> > Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> > ---
> > kernel/trace/trace_kprobe.c | 7 +------
> > kernel/trace/trace_probe.c | 8 ++++++++
> > kernel/trace/trace_uprobe.c | 5 +----
> > 3 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index d47e12596f12..5222fd82e7e4 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -221,7 +221,7 @@ static struct trace_kprobe *alloc_trace_kprobe(const char *group,
> >
> > tk->rp.maxactive = maxactive;
> >
> > - if (!event || !is_good_name(event)) {
> > + if (!event || !group) {
> > ret = -EINVAL;
> > goto error;
> > }
> > @@ -231,11 +231,6 @@ static struct trace_kprobe *alloc_trace_kprobe(const char *group,
> > if (!tk->tp.call.name)
> > goto error;
> >
> > - if (!group || !is_good_name(group)) {
> > - ret = -EINVAL;
> > - goto error;
> > - }
> > -
> > tk->tp.class.system = kstrdup(group, GFP_KERNEL);
> > if (!tk->tp.class.system)
> > goto error;
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index feae03056f0b..1f0cb4030c0b 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -172,6 +172,10 @@ int traceprobe_parse_event_name(const char **pevent, const char **pgroup,
> > return -E2BIG;
> > }
> > strlcpy(buf, event, slash - event + 1);
> > + if (!is_good_name(buf)) {
> > + pr_info("Group name must follow the rule of C identifier\n");
>
> What do you mean by "C identifier"?

I meant "the naming rules of C language identifiers". It means
that the name has to start with alphabet or "_" and only contain
alphanumeric characters and "_". Does it clear?
I couldn't think of a good word. maybe "naming convention"
does not fit...

Would you have any idea?

Thank you,

>
> -- Steve
>
> > + return -EINVAL;
> > + }
> > *pgroup = buf;
> > *pevent = slash + 1;
> > event = *pevent;
> > @@ -184,6 +188,10 @@ int traceprobe_parse_event_name(const char **pevent, const char **pgroup,
> > pr_info("Event name is too long\n");
> > return -E2BIG;
> > }
> > + if (!is_good_name(event)) {
> > + pr_info("Event name must follow the rule of C identifier\n");
> > + return -EINVAL;
> > + }
> > return 0;
> > }
> >


--
Masami Hiramatsu <mhiramat@xxxxxxxxxx>