| 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
| ||
|
Message-ID: <77f20e7.1635.196dcdc591c.Coremail.00107082@163.com>
Date: Sat, 17 May 2025 14:09:10 +0800 (CST)
From: "David Wang" <00107082@....com>
To: "Suren Baghdasaryan" <surenb@...gle.com>
Cc: akpm@...ux-foundation.org, kent.overstreet@...ux.dev, dennis@...nel.org,
tj@...nel.org, cl@...two.org, pasha.tatashin@...een.com,
linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re:[PATCH 1/1] alloc_tag: allocate percpu counters for module tags
dynamically
At 2025-05-17 08:07:39, "Suren Baghdasaryan" <surenb@...gle.com> wrote:
>When a module gets unloaded it checks whether any of its tags are still
>in use and if so, we keep the memory containing module's allocation tags
>alive until all tags are unused. However percpu counters referenced by
>the tags are freed by free_module(). This will lead to UAF if the memory
>allocated by a module is accessed after module was unloaded. To fix this
>we allocate percpu counters for module allocation tags dynamically and
>we keep it alive for tags which are still in use after module unloading.
>This also removes the requirement of a larger PERCPU_MODULE_RESERVE when
>memory allocation profiling is enabled because percpu memory for counters
>does not need to be reserved anymore.
>
>Fixes: 0db6f8d7820a ("alloc_tag: load module tags into separate contiguous memory")
>Reported-by: David Wang <00107082@....com>
>Closes: https://lore.kernel.org/all/20250516131246.6244-1-00107082@163.com/
>Signed-off-by: Suren Baghdasaryan <surenb@...gle.com>
>---
> include/linux/alloc_tag.h | 12 ++++++
> include/linux/codetag.h | 8 ++--
> include/linux/percpu.h | 4 --
> lib/alloc_tag.c | 87 +++++++++++++++++++++++++++++++--------
> lib/codetag.c | 5 ++-
> 5 files changed, 88 insertions(+), 28 deletions(-)
>
>diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
>index a946e0203e6d..8f7931eb7d16 100644
>--- a/include/linux/alloc_tag.h
>+++ b/include/linux/alloc_tag.h
>@@ -104,6 +104,16 @@ DECLARE_PER_CPU(struct alloc_tag_counters, _shared_alloc_tag);
>
> #else /* ARCH_NEEDS_WEAK_PER_CPU */
>
>+#ifdef MODULE
>+
>+#define DEFINE_ALLOC_TAG(_alloc_tag) \
>+ static struct alloc_tag _alloc_tag __used __aligned(8) \
>+ __section(ALLOC_TAG_SECTION_NAME) = { \
>+ .ct = CODE_TAG_INIT, \
>+ .counters = NULL };
>+
>+#else /* MODULE */
>+
> #define DEFINE_ALLOC_TAG(_alloc_tag) \
> static DEFINE_PER_CPU(struct alloc_tag_counters, _alloc_tag_cntr); \
> static struct alloc_tag _alloc_tag __used __aligned(8) \
>@@ -111,6 +121,8 @@ DECLARE_PER_CPU(struct alloc_tag_counters, _shared_alloc_tag);
> .ct = CODE_TAG_INIT, \
> .counters = &_alloc_tag_cntr };
>
>+#endif /* MODULE */
>+
> #endif /* ARCH_NEEDS_WEAK_PER_CPU */
>
> DECLARE_STATIC_KEY_MAYBE(CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT,
>diff --git a/include/linux/codetag.h b/include/linux/codetag.h
>index d14dbd26b370..0ee4c21c6dbc 100644
>--- a/include/linux/codetag.h
>+++ b/include/linux/codetag.h
>@@ -36,10 +36,10 @@ union codetag_ref {
> struct codetag_type_desc {
> const char *section;
> size_t tag_size;
>- void (*module_load)(struct codetag_type *cttype,
>- struct codetag_module *cmod);
>- void (*module_unload)(struct codetag_type *cttype,
>- struct codetag_module *cmod);
>+ void (*module_load)(struct module *mod,
>+ struct codetag *start, struct codetag *end);
>+ void (*module_unload)(struct module *mod,
>+ struct codetag *start, struct codetag *end);
> #ifdef CONFIG_MODULES
> void (*module_replaced)(struct module *mod, struct module *new_mod);
> bool (*needs_section_mem)(struct module *mod, unsigned long size);
>diff --git a/include/linux/percpu.h b/include/linux/percpu.h
>index 52b5ea663b9f..85bf8dd9f087 100644
>--- a/include/linux/percpu.h
>+++ b/include/linux/percpu.h
>@@ -15,11 +15,7 @@
>
> /* enough to cover all DEFINE_PER_CPUs in modules */
> #ifdef CONFIG_MODULES
>-#ifdef CONFIG_MEM_ALLOC_PROFILING
>-#define PERCPU_MODULE_RESERVE (8 << 13)
>-#else
> #define PERCPU_MODULE_RESERVE (8 << 10)
>-#endif
> #else
> #define PERCPU_MODULE_RESERVE 0
> #endif
>diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
>index 25ecc1334b67..c7f602fa7b23 100644
>--- a/lib/alloc_tag.c
>+++ b/lib/alloc_tag.c
>@@ -350,18 +350,28 @@ static bool needs_section_mem(struct module *mod, unsigned long size)
> return size >= sizeof(struct alloc_tag);
> }
>
>-static struct alloc_tag *find_used_tag(struct alloc_tag *from, struct alloc_tag *to)
>+static bool clean_unused_counters(struct alloc_tag *start_tag,
>+ struct alloc_tag *end_tag)
> {
>- while (from <= to) {
>+ struct alloc_tag *tag;
>+ bool ret = true;
>+
>+ for (tag = start_tag; tag <= end_tag; tag++) {
> struct alloc_tag_counters counter;
>
>- counter = alloc_tag_read(from);
>- if (counter.bytes)
>- return from;
>- from++;
>+ if (!tag->counters)
>+ continue;
>+
>+ counter = alloc_tag_read(tag);
>+ if (!counter.bytes) {
>+ free_percpu(tag->counters);
>+ tag->counters = NULL;
>+ } else {
>+ ret = false;
>+ }
> }
>
>- return NULL;
>+ return ret;
> }
>
> /* Called with mod_area_mt locked */
>@@ -371,12 +381,16 @@ static void clean_unused_module_areas_locked(void)
> struct module *val;
>
> mas_for_each(&mas, val, module_tags.size) {
>+ struct alloc_tag *start_tag;
>+ struct alloc_tag *end_tag;
>+
> if (val != &unloaded_mod)
> continue;
>
> /* Release area if all tags are unused */
>- if (!find_used_tag((struct alloc_tag *)(module_tags.start_addr + mas.index),
>- (struct alloc_tag *)(module_tags.start_addr + mas.last)))
>+ start_tag = (struct alloc_tag *)(module_tags.start_addr + mas.index);
>+ end_tag = (struct alloc_tag *)(module_tags.start_addr + mas.last);
>+ if (clean_unused_counters(start_tag, end_tag))
> mas_erase(&mas);
> }
> }
>@@ -561,7 +575,8 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
> static void release_module_tags(struct module *mod, bool used)
> {
> MA_STATE(mas, &mod_area_mt, module_tags.size, module_tags.size);
>- struct alloc_tag *tag;
>+ struct alloc_tag *start_tag;
>+ struct alloc_tag *end_tag;
> struct module *val;
>
> mas_lock(&mas);
>@@ -575,15 +590,22 @@ static void release_module_tags(struct module *mod, bool used)
> if (!used)
> goto release_area;
>
>- /* Find out if the area is used */
>- tag = find_used_tag((struct alloc_tag *)(module_tags.start_addr + mas.index),
>- (struct alloc_tag *)(module_tags.start_addr + mas.last));
>- if (tag) {
>- struct alloc_tag_counters counter = alloc_tag_read(tag);
>+ start_tag = (struct alloc_tag *)(module_tags.start_addr + mas.index);
>+ end_tag = (struct alloc_tag *)(module_tags.start_addr + mas.last);
>+ if (!clean_unused_counters(start_tag, end_tag)) {
>+ struct alloc_tag *tag;
>+
>+ for (tag = start_tag; tag <= end_tag; tag++) {
>+ struct alloc_tag_counters counter;
>+
>+ if (!tag->counters)
>+ continue;
>
>- pr_info("%s:%u module %s func:%s has %llu allocated at module unload\n",
>- tag->ct.filename, tag->ct.lineno, tag->ct.modname,
>- tag->ct.function, counter.bytes);
>+ counter = alloc_tag_read(tag);
>+ pr_info("%s:%u module %s func:%s has %llu allocated at module unload\n",
>+ tag->ct.filename, tag->ct.lineno, tag->ct.modname,
>+ tag->ct.function, counter.bytes);
>+ }
> } else {
> used = false;
> }
>@@ -596,6 +618,34 @@ static void release_module_tags(struct module *mod, bool used)
> mas_unlock(&mas);
> }
>
>+static void load_module(struct module *mod, struct codetag *start, struct codetag *stop)
>+{
>+ /* Allocate module alloc_tag percpu counters */
>+ struct alloc_tag *start_tag;
>+ struct alloc_tag *stop_tag;
>+ struct alloc_tag *tag;
>+
>+ if (!mod)
>+ return;
>+
>+ start_tag = ct_to_alloc_tag(start);
>+ stop_tag = ct_to_alloc_tag(stop);
>+ for (tag = start_tag; tag < stop_tag; tag++) {
>+ WARN_ON(tag->counters);
>+ tag->counters = alloc_percpu(struct alloc_tag_counters);
>+ if (!tag->counters) {
>+ while (--tag >= start_tag) {
>+ free_percpu(tag->counters);
>+ tag->counters = NULL;
>+ }
>+ shutdown_mem_profiling(true);
>+ pr_err("Failed to allocate memory for allocation tag percpu counters in the module %s. Memory allocation profiling is disabled!\n",
>+ mod->name);
>+ break;
>+ }
>+ }
>+}
>+
> static void replace_module(struct module *mod, struct module *new_mod)
> {
> MA_STATE(mas, &mod_area_mt, 0, module_tags.size);
>@@ -757,6 +807,7 @@ static int __init alloc_tag_init(void)
> .needs_section_mem = needs_section_mem,
> .alloc_section_mem = reserve_module_tags,
> .free_section_mem = release_module_tags,
>+ .module_load = load_module,
> .module_replaced = replace_module,
> #endif
> };
>diff --git a/lib/codetag.c b/lib/codetag.c
>index 42aadd6c1454..de332e98d6f5 100644
>--- a/lib/codetag.c
>+++ b/lib/codetag.c
>@@ -194,7 +194,7 @@ static int codetag_module_init(struct codetag_type *cttype, struct module *mod)
> if (err >= 0) {
> cttype->count += range_size(cttype, &range);
> if (cttype->desc.module_load)
>- cttype->desc.module_load(cttype, cmod);
>+ cttype->desc.module_load(mod, range.start, range.stop);
> }
> up_write(&cttype->mod_lock);
>
>@@ -333,7 +333,8 @@ void codetag_unload_module(struct module *mod)
> }
> if (found) {
> if (cttype->desc.module_unload)
>- cttype->desc.module_unload(cttype, cmod);
>+ cttype->desc.module_unload(cmod->mod,
>+ cmod->range.start, cmod->range.stop);
>
> cttype->count -= range_size(cttype, &cmod->range);
> idr_remove(&cttype->mod_idr, mod_id);
>
>base-commit: 6e68a205c07b3c311f19a4c9c5de95d191d5a459
>--
>2.49.0.1101.gccaa498523-goog
This patch fixes the page fault error.
Tested-by: David Wang <00107082@....com>
My tests:
1. enter recovery mode
2. install nvidia driver 570.144, failed with Unknown symbol drm_client_setup
3. modprobe drm_client_lib
4. install nvidia driver 570.144
5. install nvidia driver 550.144.03
6. reboot and repeat from step 1
I managed to run 10 rounds of above test, no abnormal detected with this patch.
And without the patch, a page fault error would happen in step 4 with very high probability.
Thanks
David
Powered by blists - more mailing lists