linux-kernel - Re: [BUG] Crash on named histogram trigger with invalid onmax variable

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: <CAP4=nvREzbs3r+Na9qF1gPBV0bRnKM5KM4qMo==daT4DuVhdGA@mail.gmail.com>
Date: Tue, 25 Feb 2025 09:01:49 +0100
From: Tomas Glozar <tglozar@...hat.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Masami Hiramatsu <mhiramat@...nel.org>, linux-trace-kernel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Tom Zanussi <zanussi@...nel.org>
Subject: Re: [BUG] Crash on named histogram trigger with invalid onmax variable

út 25. 2. 2025 v 1:52 odesílatel Steven Rostedt <rostedt@...dmis.org> napsal:
>         list_add_tail_rcu(&data->list, &file->triggers); <<<---- SKIPPED!!! (this is important!)

Ah, so the issue is that even though hist_register_trigger() has been
called, that is not where the trigger data is put into the list. In
other words, to be able to unregister a trigger properly with
hist_unregister_trigger(), you need to call hist_trigger_enable() in
addition to hist_register_trigger().

I (naively) assumed it is hist_register_trigger() that adds it to the
list, which led me to think that it is hist_trigger_match() that is
failing.

> So I moved the code around such that if event_trigger_register() succeeds,
> the next thing called is hist_trigger_enable() which adds it to the list.
> Probably should be port of event_hist_register() instead, but that could be
> part of a clean up.

I agree. One would expect that hist_register_trigger() and
hist_unregister_trigger() are paired with each other so that the
latter reverses the action of the former.

> Looks like a bunch of actions is supposed to be called if
> get_named_trigger_data() returns false. But that doesn't need to be called
> after event_trigger_register(), so moving that up shouldn't be an issue. At
> least I can't find one.

As those operate purely on the hist_data, I cannot imagine there being
a problem. The trigger isn't going to activate before being registered
anyway, it makes sense to first finish setting up whatever we don't
need the trigger data for and only then register it.

> Can you try this patch?

Yes, the patch you posted gets rid of the panic.

Tomas