linux-kernel - Re: [PATCH] tracing: Fix error handling in event_trigger

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: <20250322052645.66a0393a@batman.local.home>
Date: Sat, 22 Mar 2025 05:26:45 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Tom Zanussi <zanussi@...nel.org>
Cc: Masami Hiramatsu <mhiramat@...nel.org>, Miaoqian Lin
 <linmq006@...il.com>, Mathieu Desnoyers  <mathieu.desnoyers@...icios.com>,
 linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH] tracing: Fix error handling in event_trigger_parse

On Wed, 19 Mar 2025 14:03:03 -0500
Tom Zanussi <zanussi@...nel.org> wrote:

> In both these cases, the code calls kfree() directly in order to avoid
> the WARN_ON_ONCE(data->ref) check.
> 
> In the first case (remove), trigger_data is only being used as a test
> object and will never have data->ref incremented.
> 
> The second case is the failure case, which is also dealing with a
> trigger_data object that hasn't been successfully registered and
> therefore has a 0 data->ref.
> 
> So perhaps the event_trigger_alloc doc should be changed to something
> like:
> 
> "Use event_trigger_free() to free a successfully registered
> event_trigger_data object."

Honestly, I think event_trigger_alloc() should set the data->ref to 1,
and remove the event_trigger_init() from those that use
event_trigger_alloc(). Then it's a lot easier to map
event_trigger_free() to event_trigger_alloc() and the users don't need
to keep track of the internals of event_triggers.

Then we don't need to have special cases of error conditions after
event_trigger_alloc(), we can simply use this patch.

So, this patch should stay as is, but another patch is needed before
this to make event_trigger_alloc() set data->ref to 1, and remove the
event_trigger_init() from the callers of event_trigger_alloc().

-- Steve