From 93c76b6b2faaad7bfbc0cda840763aa4819ef26e Mon Sep 17 00:00:00 2001 From: zijun_hu Date: Wed, 5 Oct 2016 21:19:11 +0800 Subject: mm/percpu.c: correct max_distance calculation for pcpu_embed_first_chunk() pcpu_embed_first_chunk() calculates the range a percpu chunk spans into @max_distance and uses it to ensure that a chunk is not too big compared to the total vmalloc area. However, during calculation, it used incorrect top address by adding a unit size to the highest group's base address. This can make the calculated max_distance slightly smaller than the actual distance although given the scale of values involved the error is very unlikely to have an actual impact. Fix this issue by adding the group's size instead of a unit size. BTW, The type of variable max_distance is changed from size_t to unsigned long too based on below consideration: - type unsigned long usually have same width with IP core registers and can be applied at here very well - make @max_distance type consistent with the operand calculated against it such as @ai->groups[i].base_offset and macro VMALLOC_TOTAL - type unsigned long is more universal then size_t, size_t is type defined to unsigned int or unsigned long among various ARCHs usually Signed-off-by: zijun_hu Signed-off-by: Tejun Heo --- mm/percpu.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'mm/percpu.c') diff --git a/mm/percpu.c b/mm/percpu.c index 9903830aaebb..e2737e56b017 100644 --- a/mm/percpu.c +++ b/mm/percpu.c @@ -1961,7 +1961,8 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size, void *base = (void *)ULONG_MAX; void **areas = NULL; struct pcpu_alloc_info *ai; - size_t size_sum, areas_size, max_distance; + size_t size_sum, areas_size; + unsigned long max_distance; int group, i, rc; ai = pcpu_build_alloc_info(reserved_size, dyn_size, atom_size, @@ -2023,17 +2024,18 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size, } /* base address is now known, determine group base offsets */ - max_distance = 0; + i = 0; for (group = 0; group < ai->nr_groups; group++) { ai->groups[group].base_offset = areas[group] - base; - max_distance = max_t(size_t, max_distance, - ai->groups[group].base_offset); + if (areas[group] > areas[i]) + i = group; } - max_distance += ai->unit_size; + max_distance = ai->groups[i].base_offset + + ai->unit_size * ai->groups[i].nr_units; /* warn if maximum distance is further than 75% of vmalloc space */ if (max_distance > VMALLOC_TOTAL * 3 / 4) { - pr_warn("max_distance=0x%zx too large for vmalloc space 0x%lx\n", + pr_warn("max_distance=0x%lx too large for vmalloc space 0x%lx\n", max_distance, VMALLOC_TOTAL); #ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK /* and fail if we have fallback */ -- cgit v1.2.3-70-g09d2 From 9b7396624a7b503220d85428654634b60762f2b0 Mon Sep 17 00:00:00 2001 From: zijun_hu Date: Wed, 5 Oct 2016 21:30:24 +0800 Subject: mm/percpu.c: fix potential memory leakage for pcpu_embed_first_chunk() in order to ensure the percpu group areas within a chunk aren't distributed too sparsely, pcpu_embed_first_chunk() goes to error handling path when a chunk spans over 3/4 VMALLOC area, however, during the error handling, it forget to free the memory allocated for all percpu groups by going to label @out_free other than @out_free_areas. it will cause memory leakage issue if the rare scene really happens, in order to fix the issue, we check chunk spanned area immediately after completing memory allocation for all percpu groups, we go to label @out_free_areas to free the memory then return if the checking is failed. in order to verify the approach, we dump all memory allocated then enforce the jump then dump all memory freed, the result is okay after checking whether we free all memory we allocate in this function. BTW, The approach is chosen after thinking over the below scenes - we don't go to label @out_free directly to fix this issue since we maybe free several allocated memory blocks twice - the aim of jumping after pcpu_setup_first_chunk() is bypassing free usable memory other than handling error, moreover, the function does not return error code in any case, it either panics due to BUG_ON() or return 0. Signed-off-by: zijun_hu Tested-by: zijun_hu Signed-off-by: Tejun Heo --- mm/percpu.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) (limited to 'mm/percpu.c') diff --git a/mm/percpu.c b/mm/percpu.c index e2737e56b017..255714302394 100644 --- a/mm/percpu.c +++ b/mm/percpu.c @@ -1963,7 +1963,7 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size, struct pcpu_alloc_info *ai; size_t size_sum, areas_size; unsigned long max_distance; - int group, i, rc; + int group, i, highest_group, rc; ai = pcpu_build_alloc_info(reserved_size, dyn_size, atom_size, cpu_distance_fn); @@ -1979,7 +1979,8 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size, goto out_free; } - /* allocate, copy and determine base address */ + /* allocate, copy and determine base address & max_distance */ + highest_group = 0; for (group = 0; group < ai->nr_groups; group++) { struct pcpu_group_info *gi = &ai->groups[group]; unsigned int cpu = NR_CPUS; @@ -2000,6 +2001,21 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size, areas[group] = ptr; base = min(ptr, base); + if (ptr > areas[highest_group]) + highest_group = group; + } + max_distance = areas[highest_group] - base; + max_distance += ai->unit_size * ai->groups[highest_group].nr_units; + + /* warn if maximum distance is further than 75% of vmalloc space */ + if (max_distance > VMALLOC_TOTAL * 3 / 4) { + pr_warn("max_distance=0x%lx too large for vmalloc space 0x%lx\n", + max_distance, VMALLOC_TOTAL); +#ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK + /* and fail if we have fallback */ + rc = -EINVAL; + goto out_free_areas; +#endif } /* @@ -2024,24 +2040,8 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size, } /* base address is now known, determine group base offsets */ - i = 0; for (group = 0; group < ai->nr_groups; group++) { ai->groups[group].base_offset = areas[group] - base; - if (areas[group] > areas[i]) - i = group; - } - max_distance = ai->groups[i].base_offset + - ai->unit_size * ai->groups[i].nr_units; - - /* warn if maximum distance is further than 75% of vmalloc space */ - if (max_distance > VMALLOC_TOTAL * 3 / 4) { - pr_warn("max_distance=0x%lx too large for vmalloc space 0x%lx\n", - max_distance, VMALLOC_TOTAL); -#ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK - /* and fail if we have fallback */ - rc = -EINVAL; - goto out_free; -#endif } pr_info("Embedded %zu pages/cpu @%p s%zu r%zu d%zu u%zu\n", -- cgit v1.2.3-70-g09d2