linux-kernel - Re: [PATCH v3] init: staging: Fix missing warning/taint on builtin code

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: <2024071032-hardened-discard-4cf6@gregkh>
Date: Wed, 10 Jul 2024 15:22:22 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Ágatha Isabelle Chris Moreira Guedes <code@...tha.dev>
Cc: Luis Chamberlain <mcgrof@...nel.org>, linux-staging@...ts.linux.dev,
	linux-kernel@...r.kernel.org, linux-modules@...r.kernel.org,
	Uwe Kleine-König <u.kleine-koenig@...libre.com>,
	Dan Carpenter <dan.carpenter@...aro.org>, patch-reply@...tha.dev
Subject: Re: [PATCH v3] init: staging: Fix missing warning/taint on builtin
 code

On Sat, Jul 06, 2024 at 12:15:01AM -0300, Ágatha Isabelle Chris Moreira Guedes wrote:
> Fix the absence of warning message and kernel tainting when initializing
> drivers from the `drivers/staging` subtree from initcalls (when
> configured as built-in).
> 
> When such a driver is built as module and the module is loaded, the
> `load_module()` function taints the kernel to signal code of unknown
> quality is loaded, and produces a warning like this:
> 
> [    8.076352] rts5208: module is from the staging directory, the
> quality is unknown, you have been warned.
> 
> The same behaviour is absent, however, when a staging driver is compiled
> as built-in on the kernel image, since loading it happens through
> initcalls and not through load_module().
> 
> This might prevent relevant information of being available on a bug
> report (i.e. on a panic log) among other possible problems.
> 
> NOTES:
> - The patch is written in such a way that all non-staging drivers are
>   kept the way they were, except for staging drivers built with
>   `-DSTAGING_CODE`.

That's good!

> - Since it changes some macros related to clang LTO as well, I tested it
>   and it works properly in kernels compiled with both clang and gcc.

This is odd, why is it messing with LTO stuff?  It should be much more
"self contained" than this I feel like.

I see what you are doing by trying to use some of the LTO macros again,
but in doing so, it makes it really hard to understand the diff and feel
comfortable with this.

If you want to stick with what you have done here, can you split it up a
bit more?  Once patch for the LTO header file changes and then another
that only adds the staging stuff.  That way it's easier to review and
justify that nothing is going to be broken with this patch.

> - Some `checkpatch.pl` errors, warnings and checks (with `--strict`) are
>   present. Some were already there, some I introduced but I think
>   they're unavoidable. Some IMHO don´t make sense at all, I think they
>   would apply for most regular macros but initcall macros are just way
>   different.

Yeah, checkpatch and macros can get tricky, use your best judgement here
and it looks ok.

> Fixes: 061b1bd394ca ("Staging: add TAINT_CRAP for all drivers/staging code")

I think it really fixes the commit _after_ this one that turns on the
taint for the build :)

anyway, nice work, I think it's almost there!

greg k-h