Updates to mod_unload_taint::count are serialized by module_mutex. However,
readers may access this value concurrently with these updates. Since the
counter is a u64, this can result in torn reads on 32-bit architectures.

Fix the issue by changing mod_unload_taint::count to 'unsigned int' and
using WRITE_ONCE()/READ_ONCE() to access it.

Fixes: 99bd9956551b ("module: Introduce module unload taint tracking")
Signed-off-by: Petr Pavlu <[email protected]>
---
 kernel/module/internal.h |  8 +++++++-
 kernel/module/tracking.c | 12 ++++++++----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 061161cc79d9..70ef20b97c84 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -257,7 +257,13 @@ struct mod_unload_taint {
        struct list_head list;
        char name[MODULE_NAME_LEN];
        unsigned long taints;
-       u64 count;
+
+       /*
+        * Number of times a module with this name and taint combination has
+        * been unloaded. Updates are serialized by module_mutex and use
+        * WRITE_ONCE(). Readers that may run concurrently use READ_ONCE().
+        */
+       unsigned int count;
 };
 
 int try_add_tainted_module(struct module *mod);
diff --git a/kernel/module/tracking.c b/kernel/module/tracking.c
index 5bcd1c2701f0..873fceb30526 100644
--- a/kernel/module/tracking.c
+++ b/kernel/module/tracking.c
@@ -28,7 +28,10 @@ int try_add_tainted_module(struct module *mod)
                                lockdep_is_held(&module_mutex)) {
                if (!strcmp(mod_taint->name, mod->name) &&
                    mod_taint->taints == mod->taints) {
-                       mod_taint->count++;
+                       unsigned int count = mod_taint->count;
+
+                       if (count != UINT_MAX)
+                               WRITE_ONCE(mod_taint->count, count + 1);
                        goto out;
                }
        }
@@ -57,8 +60,8 @@ void print_unloaded_tainted_modules(void)
 
                        l = module_flags_taint(mod_taint->taints, buf);
                        buf[l++] = '\0';
-                       pr_cont(" %s(%s):%llu", mod_taint->name, buf,
-                               mod_taint->count);
+                       pr_cont(" %s(%s):%u", mod_taint->name, buf,
+                               READ_ONCE(mod_taint->count));
                }
        }
 }
@@ -92,7 +95,8 @@ static int unloaded_tainted_modules_seq_show(struct seq_file 
*m, void *p)
        l = module_flags_taint(mod_taint->taints, buf);
        buf[l++] = '\0';
 
-       seq_printf(m, "%s (%s) %llu", mod_taint->name, buf, mod_taint->count);
+       seq_printf(m, "%s (%s) %u", mod_taint->name, buf,
+                  READ_ONCE(mod_taint->count));
        seq_puts(m, "\n");
 
        return 0;

-- 
2.55.0

Reply via email to