linux-kernel - Re: [PATCH] trace-cmd record: Fix syntax in make_pid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CADx9qWjVny-+Sct2-pmHmZ73ohCu_6ay67-GBYP5w_gjvG0TDQ@mail.gmail.com>
Date:   Fri, 13 Oct 2017 14:32:26 -0400
From:   Will Hawkins <hawkinsw@...laugic.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     linux-kernel@...r.kernel.org, Will Hawkins <hawkinsw@...il.com>
Subject: Re: [PATCH] trace-cmd record: Fix syntax in make_pid_filter

On Fri, Oct 13, 2017 at 2:24 PM, Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Fri, 13 Oct 2017 13:57:12 -0400
> Will Hawkins <hawkinsw@...laugic.com> wrote:
>
> > In older versions of the kernel, event filtering by
> > pid does not use the set_event_pid file. Instead,
> > ftrace uses a common filter (created in make_pid_filter)
> > to filter events based on PID.
> >
> > The syntax of the filter generated by this function is
> > overly permissive. When filtering by pid, || is used
> > where && should be (in certain cases) which means that
> > unrelated events are captured.
> >
>
> Thanks for sending this.
>
> > Signed-off-by: Will Hawkins <hawkinsw@...laugic.com>
> > ---
> >  trace-record.c | 17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/trace-record.c b/trace-record.c
> > index a0b6541..d9c2959 100644
> > --- a/trace-record.c
> > +++ b/trace-record.c
> > @@ -935,14 +935,19 @@ static char *make_pid_filter(char *curr_filter, const char *field)
> >       str = filter + curr_len;
> >
> >       for (p = filter_pids; p; p = p->next) {
> > -             if (p == filter_pids)
> > -                     orit = "";
> > -             else
> > -                     orit = "||";
> > -             if (p->exclude)
> > +             if (p->exclude) {
> >                       match = "!=";
> > -             else
> > +                     if (p == filter_pids)
> > +                             orit = "";
> > +                     else
> > +                             orit = "&&";
> > +             } else {
> >                       match = "==";
> > +                     if (p == filter_pids)
> > +                             orit = "";
> > +                     else
> > +                             orit = "||";
> > +             }
>
> A little nicer way of doing this is:
>
>
>         if (p->exclude) {
>                 match = "!=";
>                 orit = "&&";
>         } else {
>                 match = "==";
>                 orit = "||";
>         }
>         if (p == filter_pids)
>                 orit = "";
>
> Care to send a v2?

Absolutely! On its way now. Sorry for not having cleaned it up
properly. I was going for the quick-and-dirty. Never thought about
collapsing the cases!

Will

>
> -- Steve
>
> >               len = sprintf(str, "%s(%s%s%d)", orit, field, match, p->pid);
> >               str += len;
> >       }
>