linux-kernel - Re: [PATCH v2 2/2] tracing: Allow execnames to be passed as args for synthetic events

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: <20210722212438.5933e714@rorschach.local.home>
Date:   Thu, 22 Jul 2021 21:24:38 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Tom Zanussi <zanussi@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>
Subject: Re: [PATCH v2 2/2] tracing: Allow execnames to be passed as args
 for synthetic events

On Fri, 23 Jul 2021 10:11:33 +0900
Masami Hiramatsu <mhiramat@...nel.org> wrote:

> I understand. As far as I can see the code, it looks a bit complicated.
> To simplify it, I need to understand the spec for "hist_field"
> for keys and for vars. And maybe need to split both case.

I'll give you a hint that took me a bit to figure out.

1) The execname is saved at the start of the histogram and not by one
of the ->fn() functions.

It's saved by hist_trigger_elt_data_init() if the elt_data->comm is
allocated. That function is part of the "tracing_map_ops" which gets
assigned by tracing_map_create() (in tracing_map.c) as the "elt_init"
function, which is called when getting a new elt element by
get_free_elt().

2) That elt_data->comm is only allocated if it finds a "hist_field"
that has HIST_FIELD_FL_EXECNAME flag set. It currently only looks for
that flag in the "keys" fields, which means that .execname is useless
for everything else. This patch changed it to search all hist_fields so
that it can find that flag if a variable has it set (which I added).

> 
> > I found this to be the least intrusive solution.
> > 
> > Maybe Tom has a better idea, but I don't have any more time to work on
> > it, and I really want this feature for the next merge window.
> > 
> > If you can make it work, and have time to play with it, I'm happy to
> > take an alternative :-)  
> 
> Me neither at least this moment, need more investigation. Let me try.

Great! I can hold off on adding this. Or I can add it, and if you come
up with a better solution, we can just swap it.

-- Steve