* Unloaded tainted modules list with tcrypt
@ 2022-10-10 15:12 Elliott, Robert (Servers)
2022-10-10 22:25 ` Aaron Tomlin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Elliott, Robert (Servers) @ 2022-10-10 15:12 UTC (permalink / raw)
To: Aaron Tomlin, linux-modules, linux-kernel
When repeatedly using the tcrypt module (which is designed to never
load successfully), the "Unloaded tainted modules" list grows forever:
Unloaded tainted modules: tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
tcrypt():1 tcrypt():1 padlock_aes():1 padlock_aes():1
isst_if_mbox_msr():1 acpi_cpufreq():1 pcc_cpufreq():1 isst_if_mbox_msr():1
acpi_cpufreq():1 pcc_cpufreq():1 acpi_cpufreq():1 acpi_cpufreq():1
pcc_cpufreq():1 isst_if_mbox_msr():1 acpi_cpufreq():1 acpi_cpufreq():1
pcc_cpufreq():1 isst_if_mbox_msr():1 acpi_cpufreq():1 acpi_cpufreq():1
isst_if_mbox_msr():1 isst_if_mbox_msr():1
acpi_cpufreq():1 pcc_cpufreq():1 acpi_cpufreq():1 isst_if_mbox_msr():1
Some other modules like acpi_cpufreq() are repeated too.
Maybe this check after the name comparison:
mod_taint->taints & mod->taints
should be:
mod_taint->taints == mod->taints
or shouldn't be there at all?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Unloaded tainted modules list with tcrypt
2022-10-10 15:12 Unloaded tainted modules list with tcrypt Elliott, Robert (Servers)
@ 2022-10-10 22:25 ` Aaron Tomlin
2022-10-10 22:38 ` Aaron Tomlin
2022-10-11 0:09 ` Elliott, Robert (Servers)
2 siblings, 0 replies; 6+ messages in thread
From: Aaron Tomlin @ 2022-10-10 22:25 UTC (permalink / raw)
To: Elliott, Robert (Servers); +Cc: linux-modules, linux-kernel
On Mon 2022-10-10 15:12 +0000, Elliott, Robert (Servers) wrote:
> When repeatedly using the tcrypt module (which is designed to never
> load successfully), the "Unloaded tainted modules" list grows forever:
>
> Unloaded tainted modules: tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 padlock_aes():1 padlock_aes():1
> isst_if_mbox_msr():1 acpi_cpufreq():1 pcc_cpufreq():1 isst_if_mbox_msr():1
> acpi_cpufreq():1 pcc_cpufreq():1 acpi_cpufreq():1 acpi_cpufreq():1
> pcc_cpufreq():1 isst_if_mbox_msr():1 acpi_cpufreq():1 acpi_cpufreq():1
> pcc_cpufreq():1 isst_if_mbox_msr():1 acpi_cpufreq():1 acpi_cpufreq():1
> isst_if_mbox_msr():1 isst_if_mbox_msr():1
> acpi_cpufreq():1 pcc_cpufreq():1 acpi_cpufreq():1 isst_if_mbox_msr():1
>
> Some other modules like acpi_cpufreq() are repeated too.
>
> Maybe this check after the name comparison:
> mod_taint->taints & mod->taints
> should be:
> mod_taint->taints == mod->taints
>
> or shouldn't be there at all?
>
Hi Elliot,
Sorry about that.
This is already addressed in linux-next:
commit a0830747f4e6810876e5f5329ce941e258e13a22
Author: Aaron Tomlin <atomlin@redhat.com>
Date: Fri Oct 7 14:32:44 2022 +0100
module: tracking: Keep a record of tainted unloaded modules only
This patch 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@gmail.com>
Fixes: 99bd9956551b ("module: Introduce module unload taint tracking")
Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
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;
+
list_for_each_entry_rcu(mod_taint, &unloaded_tainted_modules, list,
lockdep_is_held(&module_mutex)) {
if (!strcmp(mod_taint->name, mod->name) &&
Kind regards,
--
Aaron Tomlin
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Unloaded tainted modules list with tcrypt
2022-10-10 15:12 Unloaded tainted modules list with tcrypt Elliott, Robert (Servers)
2022-10-10 22:25 ` Aaron Tomlin
@ 2022-10-10 22:38 ` Aaron Tomlin
2022-10-11 0:09 ` Elliott, Robert (Servers)
2 siblings, 0 replies; 6+ messages in thread
From: Aaron Tomlin @ 2022-10-10 22:38 UTC (permalink / raw)
To: Elliott, Robert (Servers); +Cc: linux-modules, linux-kernel
On Mon 2022-10-10 15:12 +0000, Elliott, Robert (Servers) wrote:
> When repeatedly using the tcrypt module (which is designed to never
> load successfully), the "Unloaded tainted modules" list grows forever:
>
> Unloaded tainted modules: tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 padlock_aes():1 padlock_aes():1
> isst_if_mbox_msr():1 acpi_cpufreq():1 pcc_cpufreq():1 isst_if_mbox_msr():1
> acpi_cpufreq():1 pcc_cpufreq():1 acpi_cpufreq():1 acpi_cpufreq():1
> pcc_cpufreq():1 isst_if_mbox_msr():1 acpi_cpufreq():1 acpi_cpufreq():1
> pcc_cpufreq():1 isst_if_mbox_msr():1 acpi_cpufreq():1 acpi_cpufreq():1
> isst_if_mbox_msr():1 isst_if_mbox_msr():1
> acpi_cpufreq():1 pcc_cpufreq():1 acpi_cpufreq():1 isst_if_mbox_msr():1
>
> Some other modules like acpi_cpufreq() are repeated too.
>
> Maybe this check after the name comparison:
> mod_taint->taints & mod->taints
> should be:
> mod_taint->taints == mod->taints
>
> or shouldn't be there at all?
>
Elliott,
Actually, see Linus' tree i.e. commit 47cc75aa9283 ("module: tracking: Keep
a record of tainted unloaded modules only").
Kind regards,
--
Aaron Tomlin
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: Unloaded tainted modules list with tcrypt
2022-10-10 15:12 Unloaded tainted modules list with tcrypt Elliott, Robert (Servers)
2022-10-10 22:25 ` Aaron Tomlin
2022-10-10 22:38 ` Aaron Tomlin
@ 2022-10-11 0:09 ` Elliott, Robert (Servers)
2022-10-11 15:51 ` Aaron Tomlin
2 siblings, 1 reply; 6+ messages in thread
From: Elliott, Robert (Servers) @ 2022-10-11 0:09 UTC (permalink / raw)
To: Elliott, Robert (Servers), Aaron Tomlin, linux-modules, linux-kernel
> -----Original Message-----
> From: Elliott, Robert (Servers) <elliott@hpe.com>
> Sent: Monday, October 10, 2022 10:12 AM
> To: Aaron Tomlin <atomlin@redhat.com>; linux-modules@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Unloaded tainted modules list with tcrypt
>
> When repeatedly using the tcrypt module (which is designed to never
> load successfully), the "Unloaded tainted modules" list grows forever:
>
> Unloaded tainted modules: tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
> tcrypt():1 tcrypt():1 padlock_aes():1 padlock_aes():1
> isst_if_mbox_msr():1 acpi_cpufreq():1 pcc_cpufreq():1 isst_if_mbox_msr():1
> acpi_cpufreq():1 pcc_cpufreq():1 acpi_cpufreq():1 acpi_cpufreq():1
> pcc_cpufreq():1 isst_if_mbox_msr():1 acpi_cpufreq():1 acpi_cpufreq():1
> pcc_cpufreq():1 isst_if_mbox_msr():1 acpi_cpufreq():1 acpi_cpufreq():1
> isst_if_mbox_msr():1 isst_if_mbox_msr():1
> acpi_cpufreq():1 pcc_cpufreq():1 acpi_cpufreq():1 isst_if_mbox_msr():1
>
> Some other modules like acpi_cpufreq() are repeated too.
>
> Maybe this check after the name comparison:
> mod_taint->taints & mod->taints
> should be:
> mod_taint->taints == mod->taints
>
> or shouldn't be there at all?
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Unloaded tainted modules list with tcrypt
2022-10-11 0:09 ` Elliott, Robert (Servers)
@ 2022-10-11 15:51 ` Aaron Tomlin
2022-10-11 17:55 ` Elliott, Robert (Servers)
0 siblings, 1 reply; 6+ messages in thread
From: Aaron Tomlin @ 2022-10-11 15:51 UTC (permalink / raw)
To: Elliott, Robert (Servers); +Cc: linux-modules, linux-kernel
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@redhat.com>
Date: Fri Oct 7 14:38:12 2022 +0100
module: tracking: Keep a record of tainted unloaded modules only
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@gmail.com>
Fixes: 99bd9956551b ("module: Introduce module unload taint tracking")
Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
Acked-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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;
+
list_for_each_entry_rcu(mod_taint, &unloaded_tainted_modules, list,
lockdep_is_held(&module_mutex)) {
if (!strcmp(mod_taint->name, mod->name) &&
--
Aaron Tomlin
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: Unloaded tainted modules list with tcrypt
2022-10-11 15:51 ` Aaron Tomlin
@ 2022-10-11 17:55 ` Elliott, Robert (Servers)
0 siblings, 0 replies; 6+ messages in thread
From: Elliott, Robert (Servers) @ 2022-10-11 17:55 UTC (permalink / raw)
To: Aaron Tomlin; +Cc: linux-modules, linux-kernel
> -----Original Message-----
> From: Aaron Tomlin <atomlin@redhat.com>
> Sent: Tuesday, October 11, 2022 10:51 AM
> To: Elliott, Robert (Servers) <elliott@hpe.com>
> Cc: linux-modules@vger.kernel.org; linux-kernel@vger.kernel.org
> 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@redhat.com>
> 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@gmail.com>
> Fixes: 99bd9956551b ("module: Introduce module unload taint tracking")
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> Acked-by: Luis Chamberlain <mcgrof@kernel.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>
> 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".
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-11 17:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-10 15:12 Unloaded tainted modules list with tcrypt Elliott, Robert (Servers)
2022-10-10 22:25 ` Aaron Tomlin
2022-10-10 22:38 ` Aaron Tomlin
2022-10-11 0:09 ` Elliott, Robert (Servers)
2022-10-11 15:51 ` Aaron Tomlin
2022-10-11 17:55 ` Elliott, Robert (Servers)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®