Skip to contentSkip to conversation

alloc_tag: fix undetected compressed tag overflow when profiling is disabled

PATCH Superseded Hao Ge 2 participants 3 messages Aug 10 – Aug 12 1 message not in the archive

Subsystems (4)

v4 of 11 Superseded Latest is v11 →

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Hao Ge

author reviewer
#1 lore raw

PATCH v4 2/2 alloc_tag: fix undetected compressed tag overflow when profiling is disabled

In reserve_module_tags(), the tag overflow check is gated on mem_alloc_profiling_enabled():

    if (mem_alloc_profiling_enabled() && !tags_addressable())

If profiling is toggled off at runtime and a module is loaded whose tags exceed the compressed-mode limit, shutdown_mem_profiling() is skipped. vm_module_tags_populate() still maps memory for the tags and the module loads successfully, but the total tag count now exceeds what NR_UNUSED_PAGEFLAG_BITS can address.

Once profiling is re-enabled, ref_to_idx() computes each tag's index as its position in the alloc_tag array. update_page_tag_ref() masks it to alloc_tag_ref_mask before storing in page->flags. Indices beyond the mask are truncated and idx_to_ref() resolves them to wrong tags.

This silently corrupts /proc/allocinfo: allocated pages get attributed to the wrong call sites, so the statistics it reports are wrong.

mem_alloc_profiling_enabled() and mem_profiling_compressed are independent. Once compressed mode is established at boot, it stays active regardless of runtime toggles of mem_profiling.

Remove the mem_alloc_profiling_enabled() guard. On overflow, shut down profiling, release the reservation, and return -EAGAIN so that layout_and_allocate() retries with profiling disabled: codetag sections are then placed as regular module data and the module loads without profiling rather than being rejected entirely.

Skip percpu counter allocation in load_module() when profiling is disabled, as those counters would never be freed on unload.

Fixes 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
---
 kernel/module/main.c | 17 +++++++++++++++--
 mm/alloc_tag.c       | 11 ++++++++---
 2 files changed, 23 insertions(+), 5 deletions(-)
kernel/module/main.c +15 -2
index 46dd8d25a605..c32bb47a573a 100644
@@ -2971,13 +2971,26 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
 	 * this is done generically; there doesn't appear to be any
 	 * special cases for the architectures.
 	 */
+retry:
 	layout_sections(info->mod, info);
 	layout_symtab(info->mod, info);
 
 	/* Allocate and move to the final place */
 	err = move_module(info->mod, info);
-	if (err)
-		return ERR_PTR(err);
+	if (err) {
+		if (err != -EAGAIN)
+			return ERR_PTR(err);
+		/*
+		 * -EAGAIN means profiling was disabled but the module
+		 * can still load without it. Reset state and retry.
+		 */
+		rewrite_section_headers(info, flags);
+		for_each_mod_mem_type(type)
+			info->mod->mem[type].size = 0;
+		info->sechdrs[info->index.sym].sh_flags &= ~(unsigned long)SHF_ALLOC;
+		info->sechdrs[info->index.str].sh_flags &= ~(unsigned long)SHF_ALLOC;
+		goto retry;
+	}
 
 	/* Module has been copied to its final place now: return it. */
 	mod = (void *)info->sechdrs[info->index.mod].sh_addr;
mm/alloc_tag.c +8 -3
index af44f90379f2..3eba6331174d 100644
@@ -950,10 +950,12 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
 		int grow_res;
 
 		module_tags.size = offset + size;
-		if (mem_alloc_profiling_enabled() && !tags_addressable()) {
+		if (!tags_addressable()) {
 			shutdown_mem_profiling(true);
-			pr_warn("With module %s there are too many tags to fit in %d page flag bits. Memory allocation profiling is disabled!\n",
-				mod->name, NR_UNUSED_PAGEFLAG_BITS);
+			pr_warn_once("With module %s there are too many tags to fit in %d page flag bits. Memory allocation profiling is disabled!\n",
+				     mod->name, NR_UNUSED_PAGEFLAG_BITS);
+			release_module_tags(mod, false);
+			return ERR_PTR(-EAGAIN);
 		}
 
 		grow_res = vm_module_tags_populate();
@@ -975,6 +977,9 @@ static int load_module(struct module *mod, struct codetag *start, struct codetag
 	struct alloc_tag *stop_tag;
 	struct alloc_tag *tag;
 
+	if (!mem_profiling_support)
+		return 0;
+
 	/* percpu counters for core allocations are already statically allocated */
 	if (!mod)
 		return 0;
Signature · 1 line
2.25.1
#2 lore raw
Quoted 25 lines · Hao Ge

In reserve_module_tags(), the tag overflow check is gated on mem_alloc_profiling_enabled():

    if (mem_alloc_profiling_enabled() && !tags_addressable())

If profiling is toggled off at runtime and a module is loaded whose tags exceed the compressed-mode limit, shutdown_mem_profiling() is skipped. vm_module_tags_populate() still maps memory for the tags and the module loads successfully, but the total tag count now exceeds what NR_UNUSED_PAGEFLAG_BITS can address.

Once profiling is re-enabled, ref_to_idx() computes each tag's index as its position in the alloc_tag array. update_page_tag_ref() masks it to alloc_tag_ref_mask before storing in page->flags. Indices beyond the mask are truncated and idx_to_ref() resolves them to wrong tags.

This silently corrupts /proc/allocinfo: allocated pages get attributed to the wrong call sites, so the statistics it reports are wrong.

mem_alloc_profiling_enabled() and mem_profiling_compressed are independent. Once compressed mode is established at boot, it stays active regardless of runtime toggles of mem_profiling.

Quoted 8 lines

Remove the mem_alloc_profiling_enabled() guard. On overflow, shut down profiling, release the reservation, and return -EAGAIN so that layout_and_allocate() retries with profiling disabled: codetag sections are then placed as regular module data and the module loads without profiling rather than being rejected entirely.

Skip percpu counter allocation in load_module() when profiling is disabled, as those counters would never be freed on unload.

The approach looks correct but we need to address Sashiko's findings. I'll focus on the pre-existing issues and will post fixes later today or tomorrow.

Quoted 76 lines
Fixes 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
---
 kernel/module/main.c | 17 +++++++++++++++--
 mm/alloc_tag.c       | 11 ++++++++---
 2 files changed, 23 insertions(+), 5 deletions(-)
kernel/module/main.c +15 -2
index 46dd8d25a605..c32bb47a573a 100644
@@ -2971,13 +2971,26 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
         * this is done generically; there doesn't appear to be any
         * special cases for the architectures.
         */
+retry:
        layout_sections(info->mod, info);
        layout_symtab(info->mod, info);
 
        /* Allocate and move to the final place */
        err = move_module(info->mod, info);
-       if (err)
-               return ERR_PTR(err);
+       if (err) {
+               if (err != -EAGAIN)
+                       return ERR_PTR(err);
+               /*
+                * -EAGAIN means profiling was disabled but the module
+                * can still load without it. Reset state and retry.
+                */
+               rewrite_section_headers(info, flags);
+               for_each_mod_mem_type(type)
+                       info->mod->mem[type].size = 0;
+               info->sechdrs[info->index.sym].sh_flags &= ~(unsigned long)SHF_ALLOC;
+               info->sechdrs[info->index.str].sh_flags &= ~(unsigned long)SHF_ALLOC;
+               goto retry;
+       }
 
        /* Module has been copied to its final place now: return it. */
        mod = (void *)info->sechdrs[info->index.mod].sh_addr;
mm/alloc_tag.c +8 -4
index af44f90379f2..3eba6331174d 100644
@@ -950,10 +950,12 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
                int grow_res;
 
                module_tags.size = offset + size;
-               if (mem_alloc_profiling_enabled() && !tags_addressable()) {
+               if (!tags_addressable()) {
                        shutdown_mem_profiling(true);
-                       pr_warn("With module %s there are too many tags to fit in %d page flag bits. Memory allocation profiling is disabled!\n",
-                               mod->name, NR_UNUSED_PAGEFLAG_BITS);
+                       pr_warn_once("With module %s there are too many tags to fit in %d page flag bits. Memory allocation profiling is disabled!\n",
+                                    mod->name, NR_UNUSED_PAGEFLAG_BITS);
+                       release_module_tags(mod, false);
+                       return ERR_PTR(-EAGAIN);
                }
 
                grow_res = vm_module_tags_populate();
@@ -975,6 +977,9 @@ static int load_module(struct module *mod, struct codetag *start, struct codetag
        struct alloc_tag *stop_tag;
        struct alloc_tag *tag;
 
+       if (!mem_profiling_support)
+               return 0;
+
        /* percpu counters for core allocations are already statically allocated */
        if (!mod)
                return 0;
--

2.25.1

#3 lore raw
Quoted 33 lines · Suren Baghdasaryan
Quoted 31 lines · Hao Ge

In reserve_module_tags(), the tag overflow check is gated on mem_alloc_profiling_enabled():

    if (mem_alloc_profiling_enabled() && !tags_addressable())

If profiling is toggled off at runtime and a module is loaded whose tags exceed the compressed-mode limit, shutdown_mem_profiling() is skipped. vm_module_tags_populate() still maps memory for the tags and the module loads successfully, but the total tag count now exceeds what NR_UNUSED_PAGEFLAG_BITS can address.

Once profiling is re-enabled, ref_to_idx() computes each tag's index as its position in the alloc_tag array. update_page_tag_ref() masks it to alloc_tag_ref_mask before storing in page->flags. Indices beyond the mask are truncated and idx_to_ref() resolves them to wrong tags.

This silently corrupts /proc/allocinfo: allocated pages get attributed to the wrong call sites, so the statistics it reports are wrong.

mem_alloc_profiling_enabled() and mem_profiling_compressed are independent. Once compressed mode is established at boot, it stays active regardless of runtime toggles of mem_profiling.

Remove the mem_alloc_profiling_enabled() guard. On overflow, shut down profiling, release the reservation, and return -EAGAIN so that layout_and_allocate() retries with profiling disabled: codetag sections are then placed as regular module data and the module loads without profiling rather than being rejected entirely.

Quoted 6 lines
Quoted 2 lines

Skip percpu counter allocation in load_module() when profiling is disabled, as those counters would never be freed on unload.

The approach looks correct but we need to address Sashiko's findings. I'll focus on the pre-existing issues and will post fixes later today or tomorrow.

Sorry, I'll need some more time for these. Day job got in the way :/ I'll try to get it done by end of the week.

Quoted 77 lines
Quoted 77 lines
Fixes 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
---
 kernel/module/main.c | 17 +++++++++++++++--
 mm/alloc_tag.c       | 11 ++++++++---
 2 files changed, 23 insertions(+), 5 deletions(-)
kernel/module/main.c +15 -2
index 46dd8d25a605..c32bb47a573a 100644
@@ -2971,13 +2971,26 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
         * this is done generically; there doesn't appear to be any
         * special cases for the architectures.
         */
+retry:
        layout_sections(info->mod, info);
        layout_symtab(info->mod, info);
 
        /* Allocate and move to the final place */
        err = move_module(info->mod, info);
-       if (err)
-               return ERR_PTR(err);
+       if (err) {
+               if (err != -EAGAIN)
+                       return ERR_PTR(err);
+               /*
+                * -EAGAIN means profiling was disabled but the module
+                * can still load without it. Reset state and retry.
+                */
+               rewrite_section_headers(info, flags);
+               for_each_mod_mem_type(type)
+                       info->mod->mem[type].size = 0;
+               info->sechdrs[info->index.sym].sh_flags &= ~(unsigned long)SHF_ALLOC;
+               info->sechdrs[info->index.str].sh_flags &= ~(unsigned long)SHF_ALLOC;
+               goto retry;
+       }
 
        /* Module has been copied to its final place now: return it. */
        mod = (void *)info->sechdrs[info->index.mod].sh_addr;
mm/alloc_tag.c +8 -4
index af44f90379f2..3eba6331174d 100644
@@ -950,10 +950,12 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
                int grow_res;
 
                module_tags.size = offset + size;
-               if (mem_alloc_profiling_enabled() && !tags_addressable()) {
+               if (!tags_addressable()) {
                        shutdown_mem_profiling(true);
-                       pr_warn("With module %s there are too many tags to fit in %d page flag bits. Memory allocation profiling is disabled!\n",
-                               mod->name, NR_UNUSED_PAGEFLAG_BITS);
+                       pr_warn_once("With module %s there are too many tags to fit in %d page flag bits. Memory allocation profiling is disabled!\n",
+                                    mod->name, NR_UNUSED_PAGEFLAG_BITS);
+                       release_module_tags(mod, false);
+                       return ERR_PTR(-EAGAIN);
                }
 
                grow_res = vm_module_tags_populate();
@@ -975,6 +977,9 @@ static int load_module(struct module *mod, struct codetag *start, struct codetag
        struct alloc_tag *stop_tag;
        struct alloc_tag *tag;
 
+       if (!mem_profiling_support)
+               return 0;
+
        /* percpu counters for core allocations are already statically allocated */
        if (!mod)
                return 0;
--

2.25.1