RE: Unloaded tainted modules list with tcrypt

[Elliott, Robert (Servers)]

> -----Original Message-----
> From: Aaron Tomlin <atomlin@xxxxxxxxxx>
> Sent: Tuesday, October 11, 2022 10:51 AM
> To: Elliott, Robert (Servers) <elliott@xxxxxxx>
> Cc: linux-modules@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: Unloaded tainted modules list with tcrypt
> 
> On Tue 2022-10-11 00:09 +0000, Elliott, Robert (Servers) wrote:
> > Changing to == seems to work well - that shows incremented counts
> > rather than new entries:
> > Unloaded tainted modules: tcrypt():40 padlock_aes():1 isst_if_mbox_msr():56
> pcc_cpufreq():56 acpi_cpufreq():112
> 
> Hi Elliott,
> 
> Please note, any module that does not carry a taint should _not_
> be on the aforementioned list. See the following which is already
> in Linus' tree:
> 
> commit 47cc75aa92837a9d3f15157d6272ff285585d75d
> Author: Aaron Tomlin <atomlin@xxxxxxxxxx>
> Date:   Fri Oct 7 14:38:12 2022 +0100
> 
>     module: tracking: Keep a record of tainted unloaded modules only

Thanks. With that change, the list remains blank as tcrypt and the
others are never added.

The counts of failed loads are mildly interesting (e.g., cpufreq
attempts are apparently based on the number of CPU cores present), but
there are other ways to observe that.

>     This ensures that no module record/or entry is added to the
>     unloaded_tainted_modules list if it does not carry a taint.
> 
>     Reported-by: Alexey Dobriyan <adobriyan@xxxxxxxxx>
>     Fixes: 99bd9956551b ("module: Introduce module unload taint tracking")
>     Signed-off-by: Aaron Tomlin <atomlin@xxxxxxxxxx>
>     Acked-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
>     Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> 
> diff --git a/kernel/module/tracking.c b/kernel/module/tracking.c
> index a139e63b6f20..26d812e07615 100644
> --- a/kernel/module/tracking.c
> +++ b/kernel/module/tracking.c
> @@ -22,6 +22,9 @@ int try_add_tainted_module(struct module *mod)
> 
>  	module_assert_mutex_or_preempt();
> 
> +	if (!mod->taints)
> +		goto out;
> +

One nit - since the out label just leads to a return statement,
the gotos in the function would be simpler as "return 0".