linux-kernel - Re: [RFC PATCH RESEND] tracing: add kernel documentation for trace_array_set_clr_event, trace_set_clr

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: <CA+wEVJZRFGW-8-FT9qJJcaWZ7So+tFeAdyupoOMG4ev_8Cpqdw@mail.gmail.com>
Date: Wed, 9 Jul 2025 16:49:39 +0200
From: Gabriele Paoloni <gpaoloni@...hat.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: "Masami Hiramatsu (Google)" <mhiramat@...nel.org>, mathieu.desnoyers@...icios.com, 
	linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [RFC PATCH RESEND] tracing: add kernel documentation for
 trace_array_set_clr_event, trace_set_clr_event and supporting functions

On Wed, Jul 9, 2025 at 4:12 PM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Wed, 9 Jul 2025 15:35:50 +0200
> Gabriele Paoloni <gpaoloni@...hat.com> wrote:
>
> > > Hmm, now here's an interesting point. So this is to define requirements
> > > of a function based on what the function is doing. But does the
> > > function have to have strict requirements?
> >
> > IMO one of the main goals for these requirements is testability.
> > In order to have testable requirements we should state what the
> > valid input values are. In this case:
> > 0 -> disable, 1 -> enable, everything else -> Error.
> >
> > Now checking the code again it seems that the switch statement
> > is missing a default "ret = -EINVAL" (or else it should be changed
> > to boolean, but I guess it would have a wider impact on the rest
> > of the code...).
>
> Well, it's mostly used internally and the only places that call it uses
> 0 or 1, so there's never been any issue.
>
> >
> > >
> > > If it can handle "0" or "!0" does that mean that's how it will be
> > > defined? Or can it just state "0" or "1" and yes "2" is UB. That is,
> > > it's not part of the requirements but if someone passes in 2, it could
> > > act as a 1 as it's UB and implementation defined. Not a requirement.
> >
> > Right now if 2 is passed the function would silently return success,
> > but do we have a use case for this? I am trying to understand
> > where the implementation defined behavior would be....
>
> The issue is that all the callers pass in the proper value, and that
> can be easily verified, but by adding the "anything else ERROR", it
> would require adding more code that is not needed.
>
> I rather just switch that and soft_disable into a boolean than to add
> superficial error checks.

Many thanks for the explanation. Please consider that, from a requirement
point of view, it is also perfectly fine to document the assumption of use that
no other values than 0 or 1 shall be passed by the caller (so the bool rework
can be avoided eventually).

Gab

>
>
> -- Steve
>