linux-kernel - Re: [PATCH v2 8/9] eventfs: moving tracing/events to eventfs

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <400E4466-629C-4306-86B1-E8BA0CB5EF66@vmware.com>
Date:   Wed, 10 May 2023 11:11:15 +0000
From:   Ajay Kaher <akaher@...are.com>
To:     Steven Rostedt <rostedt@...dmis.org>
CC:     kernel test robot <lkp@...el.com>,
        "mhiramat@...nel.org" <mhiramat@...nel.org>,
        "shuah@...nel.org" <shuah@...nel.org>,
        "llvm@...ts.linux.dev" <llvm@...ts.linux.dev>,
        "oe-kbuild-all@...ts.linux.dev" <oe-kbuild-all@...ts.linux.dev>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-trace-kernel@...r.kernel.org" 
        <linux-trace-kernel@...r.kernel.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
        "chinglinyu@...gle.com" <chinglinyu@...gle.com>,
        Nadav Amit <namit@...are.com>,
        Srivatsa Bhat <srivatsab@...are.com>,
        "srivatsa@...il.mit.edu" <srivatsa@...il.mit.edu>,
        Alexey Makhalov <amakhalov@...are.com>,
        Vasavi Sirnapalli <vsirnapalli@...are.com>,
        Tapas Kundu <tkundu@...are.com>,
        "er.ajay.kaher@...il.com" <er.ajay.kaher@...il.com>
Subject: Re: [PATCH v2 8/9] eventfs: moving tracing/events to eventfs



> On 09-May-2023, at 10:15 PM, Steven Rostedt <rostedt@...dmis.org> wrote:
> 
> !! External Email
> 
> On Tue, 9 May 2023 12:29:23 +0000
> Ajay Kaher <akaher@...are.com> wrote:
> 
>> > On 02/05/23, 11:42 PM, "kernel test robot" <lkp@...el.com> wrote:
>>>>> kernel/trace/trace_events.c:2424:17: warning: variable 'd_events' set but not used [-Wunused-but-set-variable]
>>>          struct dentry *d_events;
>>>                      ^
>>>   1 warning generated.
>>> 
>> 
>> Steve, with-in event_create_dir(), do we have any scenario when file->event_call->class->system
>> doesn't have TRACE_SYSTEM? And need to execute following:
>> 
>> ae63b31e4d0e2e Steven Rostedt           2012-05-03  2437                d_events = parent;
>> 
>> looking for your input if we could remove d_events from event_create_dir().
>> 
> 
> I have hit this in the beginning, but I don't think it's an issue
> anymore. Perhaps just have it be:
> 
>        if (WARN_ON_ONCE(strcmp(call->class->system, TRACE_SYSTEM) == 0))
>                return -ENODEV;
> 
>        ef_subsystem = event_subsystem_dir(tr, call->class->system, file, parent);
> 
> Hmm, how about just add this patch before your patch set:
> 

Sounds good. Thanks Steve :)
Once you will merge below patch, I will rebase this patch in v3.

- Ajay

> 
> From: "Steven Rostedt (Google)" <rostedt@...dmis.org>
> Subject: [PATCH] tracing: Require all trace events to have a TRACE_SYSTEM
> 
> The creation of the trace event directory requires that a TRACE_SYSTEM is
> defined that the trace event directory is added within the system it was
> defined in.
> 
> The code handled the case where a TRACE_SYSTEM was not added, and would
> then add the event at the events directory. But nothing should be doing
> this. This code also prevents the implementation of creating dynamic
> dentrys for the eventfs system.
> 
> As this path has never been hit on correct code, remove it. If it does get
> hit, issues a WARN_ON_ONCE() and return ENODEV.
> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> ---
> 
>  (lightly tested)
> 
> kernel/trace/trace_events.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 654ffa40457a..16bc5ba45507 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -2424,14 +2424,15 @@ event_create_dir(struct dentry *parent, struct trace_event_file *file)
> 
>        /*
>         * If the trace point header did not define TRACE_SYSTEM
> -        * then the system would be called "TRACE_SYSTEM".
> +        * then the system would be called "TRACE_SYSTEM". This should
> +        * never happen.
>         */
> -       if (strcmp(call->class->system, TRACE_SYSTEM) != 0) {
> -               d_events = event_subsystem_dir(tr, call->class->system, file, parent);
> -               if (!d_events)
> -                       return -ENOMEM;
> -       } else
> -               d_events = parent;
> +       if (WARN_ON_ONCE(strcmp(call->class->system, TRACE_SYSTEM) == 0))
> +               return -ENODEV;
> +
> +       d_events = event_subsystem_dir(tr, call->class->system, file, parent);
> +       if (!d_events)
> +               return -ENOMEM;
> 
>        name = trace_event_name(call);
>        file->dir = tracefs_create_dir(name, d_events);
> --
> 2.39.2
> 
> 
> !! External Email: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender.