memblock: introduce saner 'memblock_free_ptr()' interface
authorLinus Torvalds <torvalds@linux-foundation.org>
Tue, 14 Sep 2021 20:23:22 +0000 (13:23 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Tue, 14 Sep 2021 20:23:22 +0000 (13:23 -0700)
The boot-time allocation interface for memblock is a mess, with
'memblock_alloc()' returning a virtual pointer, but then you are
supposed to free it with 'memblock_free()' that takes a _physical_
address.

Not only is that all kinds of strange and illogical, but it actually
causes bugs, when people then use it like a normal allocation function,
and it fails spectacularly on a NULL pointer:

   https://lore.kernel.org/all/20210912140820.GD25450@xsang-OptiPlex-9020/

or just random memory corruption if the debug checks don't catch it:

   https://lore.kernel.org/all/61ab2d0c-3313-aaab-514c-e15b7aa054a0@suse.cz/

I really don't want to apply patches that treat the symptoms, when the
fundamental cause is this horribly confusing interface.

I started out looking at just automating a sane replacement sequence,
but because of this mix or virtual and physical addresses, and because
people have used the "__pa()" macro that can take either a regular
kernel pointer, or just the raw "unsigned long" address, it's all quite
messy.

So this just introduces a new saner interface for freeing a virtual
address that was allocated using 'memblock_alloc()', and that was kept
as a regular kernel pointer.  And then it converts a couple of users
that are obvious and easy to test, including the 'xbc_nodes' case in
lib/bootconfig.c that caused problems.

Reported-by: kernel test robot <oliver.sang@intel.com>
Fixes: 40caa127f3c7 ("init: bootconfig: Remove all bootconfig data when the init memory is removed")
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
arch/x86/kernel/setup_percpu.c
arch/x86/mm/kasan_init_64.c
arch/x86/mm/numa.c
arch/x86/mm/numa_emulation.c
drivers/base/arch_numa.c
drivers/macintosh/smu.c
include/linux/memblock.h
init/main.c
kernel/printk/printk.c
lib/bootconfig.c
mm/memblock.c

index 78a32b956e8104237c15f72ebf58460578878dea..5afd985591939cc285f0b035cc2e2fc810cef4e2 100644 (file)
@@ -135,7 +135,7 @@ static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size, size_t align)
 
 static void __init pcpu_fc_free(void *ptr, size_t size)
 {
-       memblock_free(__pa(ptr), size);
+       memblock_free_ptr(ptr, size);
 }
 
 static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
index 1a50434c8a4dab44d6d23742d3bcd2dbbc1e2d99..ef885370719a605876b3375e22db72f8d025cb64 100644 (file)
@@ -49,8 +49,7 @@ static void __init kasan_populate_pmd(pmd_t *pmd, unsigned long addr,
                        p = early_alloc(PMD_SIZE, nid, false);
                        if (p && pmd_set_huge(pmd, __pa(p), PAGE_KERNEL))
                                return;
-                       else if (p)
-                               memblock_free(__pa(p), PMD_SIZE);
+                       memblock_free_ptr(p, PMD_SIZE);
                }
 
                p = early_alloc(PAGE_SIZE, nid, true);
@@ -86,8 +85,7 @@ static void __init kasan_populate_pud(pud_t *pud, unsigned long addr,
                        p = early_alloc(PUD_SIZE, nid, false);
                        if (p && pud_set_huge(pud, __pa(p), PAGE_KERNEL))
                                return;
-                       else if (p)
-                               memblock_free(__pa(p), PUD_SIZE);
+                       memblock_free_ptr(p, PUD_SIZE);
                }
 
                p = early_alloc(PAGE_SIZE, nid, true);
index a1b5c71099e61d50095666e18e16e6b921c5d664..1e9b93b088dbf0dc454d2ebb30dcab055d1f7f59 100644 (file)
@@ -355,7 +355,7 @@ void __init numa_reset_distance(void)
 
        /* numa_distance could be 1LU marking allocation failure, test cnt */
        if (numa_distance_cnt)
-               memblock_free(__pa(numa_distance), size);
+               memblock_free_ptr(numa_distance, size);
        numa_distance_cnt = 0;
        numa_distance = NULL;   /* enable table creation */
 }
index 737491b13728c3ba88146d8ba728abbc2cdd92db..e801e30089c436087681b8f09ad88e76d3046d56 100644 (file)
@@ -517,8 +517,7 @@ void __init numa_emulation(struct numa_meminfo *numa_meminfo, int numa_dist_cnt)
        }
 
        /* free the copied physical distance table */
-       if (phys_dist)
-               memblock_free(__pa(phys_dist), phys_size);
+       memblock_free_ptr(phys_dist, phys_size);
        return;
 
 no_emu:
index 46c503486e9629a1786c63f14f80405da8f468a7..00fb4120a5b3a8166882216b91316d86ab0c9950 100644 (file)
@@ -264,7 +264,7 @@ void __init numa_free_distance(void)
        size = numa_distance_cnt * numa_distance_cnt *
                sizeof(numa_distance[0]);
 
-       memblock_free(__pa(numa_distance), size);
+       memblock_free_ptr(numa_distance, size);
        numa_distance_cnt = 0;
        numa_distance = NULL;
 }
index 94fb63a7b357372a890c95da371964739610f03d..fe63d5ee201b866c0e0681b9605e60ba4aaf2720 100644 (file)
@@ -570,7 +570,7 @@ fail_msg_node:
 fail_db_node:
        of_node_put(smu->db_node);
 fail_bootmem:
-       memblock_free(__pa(smu), sizeof(struct smu_device));
+       memblock_free_ptr(smu, sizeof(struct smu_device));
        smu = NULL;
 fail_np:
        of_node_put(np);
index b066024c62e3f4937849c0fa5ac0d7091166160a..34de69b3b8badcdf971cd3f34b805d1b5b23f52d 100644 (file)
@@ -118,6 +118,7 @@ int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
 int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
 
 void memblock_free_all(void);
+void memblock_free_ptr(void *ptr, size_t size);
 void reset_node_managed_pages(pg_data_t *pgdat);
 void reset_all_zones_managed_pages(void);
 
index 5c9a48df90e1b499f88299e6410f606b879072ff..3f721693444122243c8992d0892cf5f24d68d093 100644 (file)
@@ -924,7 +924,7 @@ static void __init print_unknown_bootoptions(void)
                end += sprintf(end, " %s", *p);
 
        pr_notice("Unknown command line parameters:%s\n", unknown_options);
-       memblock_free(__pa(unknown_options), len);
+       memblock_free_ptr(unknown_options, len);
 }
 
 asmlinkage __visible void __init __no_sanitize_address start_kernel(void)
index 825277e1e742ddfaacd6cc99f4ed7a4156d88778..a8d0a58deebc708b08574bee0e29bbbe5ff961b1 100644 (file)
@@ -1166,9 +1166,9 @@ void __init setup_log_buf(int early)
        return;
 
 err_free_descs:
-       memblock_free(__pa(new_descs), new_descs_size);
+       memblock_free_ptr(new_descs, new_descs_size);
 err_free_log_buf:
-       memblock_free(__pa(new_log_buf), new_log_buf_len);
+       memblock_free_ptr(new_log_buf, new_log_buf_len);
 }
 
 static bool __read_mostly ignore_loglevel;
index f8419cff11471c94874a5e20f72ef32abae7a5d6..5ae248b2937389363c9519954a14f86877def4c7 100644 (file)
@@ -792,7 +792,7 @@ void __init xbc_destroy_all(void)
        xbc_data = NULL;
        xbc_data_size = 0;
        xbc_node_num = 0;
-       memblock_free(__pa(xbc_nodes), sizeof(struct xbc_node) * XBC_NODE_MAX);
+       memblock_free_ptr(xbc_nodes, sizeof(struct xbc_node) * XBC_NODE_MAX);
        xbc_nodes = NULL;
        brace_index = 0;
 }
index 0ab5a749bfa69e07f124e9d378f8468e459053de..184dcd2e5d9987a41315ebcf53d1dfa127e59913 100644 (file)
@@ -472,7 +472,7 @@ static int __init_memblock memblock_double_array(struct memblock_type *type,
                kfree(old_array);
        else if (old_array != memblock_memory_init_regions &&
                 old_array != memblock_reserved_init_regions)
-               memblock_free(__pa(old_array), old_alloc_size);
+               memblock_free_ptr(old_array, old_alloc_size);
 
        /*
         * Reserve the new array if that comes from the memblock.  Otherwise, we
@@ -795,6 +795,20 @@ int __init_memblock memblock_remove(phys_addr_t base, phys_addr_t size)
        return memblock_remove_range(&memblock.memory, base, size);
 }
 
+/**
+ * memblock_free_ptr - free boot memory allocation
+ * @ptr: starting address of the  boot memory allocation
+ * @size: size of the boot memory block in bytes
+ *
+ * Free boot memory block previously allocated by memblock_alloc_xx() API.
+ * The freeing memory will not be released to the buddy allocator.
+ */
+void __init_memblock memblock_free_ptr(void *ptr, size_t size)
+{
+       if (ptr)
+               memblock_free(__pa(ptr), size);
+}
+
 /**
  * memblock_free - free boot memory block
  * @base: phys starting address of the  boot memory block