[RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation
From: Mike Rapoport rppt@linux.ibm.com
Marc Gonzalez reported the following kmemleak crash:
Unable to handle kernel paging request at virtual address ffffffc021e00000 Mem abort info: ESR = 0x96000006 Exception class = DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 Data abort info: ISV = 0, ISS = 0x00000006 CM = 0, WnR = 0 swapper pgtable: 4k pages, 39-bit VAs, pgdp = (____ptrval____) [ffffffc021e00000] pgd=000000017e3ba803, pud=000000017e3ba803, pmd=0000000000000000 Internal error: Oops: 96000006 [#1] PREEMPT SMP Modules linked in: CPU: 6 PID: 523 Comm: kmemleak Tainted: G S W 5.0.0-rc1 #13 Hardware name: Qualcomm Technologies, Inc. MSM8998 v1 MTP (DT) pstate: 80000085 (Nzcv daIf -PAN -UAO) pc : scan_block+0x70/0x190 lr : scan_block+0x6c/0x190 sp : ffffff8012e8bd20 x29: ffffff8012e8bd20 x28: ffffffc0fdbaf018 x27: ffffffc022000000 x26: 0000000000000080 x25: ffffff8011aadf70 x24: ffffffc0f8cc8000 x23: ffffff8010dc8000 x22: ffffff8010dc8830 x21: ffffffc021e00ff9 x20: ffffffc0f8cc8050 x19: ffffffc021e00000 x18: 0000000000002409 x17: 0000000000000200 x16: 0000000000000000 x15: ffffff8010e14dd8 x14: 0000000000002406 x13: 000000004c4dd0c6 x12: ffffffc0f77dad58 x11: 0000000000000001 x10: ffffff8010d9e688 x9 : ffffff8010d9f000 x8 : ffffff8010d9e688 x7 : 0000000000000002 x6 : 0000000000000000 x5 : ffffff8011511c20 x4 : 00000000000026d1 x3 : ffffff8010e14d88 x2 : 5b36396f4e7d4000 x1 : 0000000000208040 x0 : 0000000000000000 Process kmemleak (pid: 523, stack limit = 0x(____ptrval____)) Call trace: scan_block+0x70/0x190 scan_gray_list+0x108/0x1c0 kmemleak_scan+0x33c/0x7c0 kmemleak_scan_thread+0x98/0xf0 kthread+0x11c/0x120 ret_from_fork+0x10/0x1c Code: f9000fb4 d503201f 97ffffd2 35000580 (f9400260) ---[ end trace 176d6ed9d86a0c33 ]--- note: kmemleak[523] exited with preempt_count 2
The crash happens when a no-map area is allocated in early_init_dt_alloc_reserved_memory_arch(). The allocated region is registered with kmemleak, but it is then removed from memblock using memblock_remove() that is not kmemleak-aware.
Replacing __memblock_alloc_base() with memblock_find_in_range() makes sure that the allocated memory is not added to kmemleak and then memblock_remove()'ing this memory is safe.
As a bonus, since memblock_find_in_range() ensures the allocation in the specified range, the bounds check can be removed.
Cc: stable@vger.kernel.org # 3.15+ Fixes: 3f0c820664483 ("drivers: of: add initialization code for dynamic reserved memory") Acked-by: Marek Szyprowski m.szyprowski@samsung.com Acked-by: Prateek Patel prpatel@nvidia.com Tested-by: Marc Gonzalez marc.w.gonzalez@free.fr Signed-off-by: Mike Rapoport rppt@linux.ibm.com --- Resend with DT CCed to reach robh's patch queue I added CC: stable, Fixes, and Prateek's ack Trim recipients list to minimize inconvenience --- drivers/of/of_reserved_mem.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 1977ee0adcb1..2ae81604ffef 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -31,27 +31,19 @@ int __init __weak early_init_dt_alloc_reserved_memory_arch(phys_addr_t size, phys_addr_t *res_base) { phys_addr_t base; - /* - * We use __memblock_alloc_base() because memblock_alloc_base() - * panic()s on allocation failure. - */ + end = !end ? MEMBLOCK_ALLOC_ANYWHERE : end; align = !align ? SMP_CACHE_BYTES : align; - base = __memblock_alloc_base(size, align, end); + base = memblock_find_in_range(size, align, start, end); if (!base) return -ENOMEM;
- /* - * Check if the allocated region fits in to start..end window - */ - if (base < start) { - memblock_free(base, size); - return -ENOMEM; - } - *res_base = base; if (nomap) return memblock_remove(base, size); + else + return memblock_reserve(base, size); + return 0; }
On 04/02/2019 15:37, Marc Gonzalez wrote:
Mike, Stephen,
I'm confused over commit 3532b3b554a216f30edb841d29eef48521bdc592 in linux-next "memblock: drop __memblock_alloc_base()"
It's definitely going to conflict with the proposed patch over drivers/of/of_reserved_mem.c
Rob, what's the next step then?
Regards.
On Mon, Feb 11, 2019 at 10:47 AM Marc Gonzalez marc.w.gonzalez@free.fr wrote:
Rebase it on top of what's in linux-next and apply it to the tree which has the above dependency. I'm guessing that is Andrew Morton's tree.
Rob
Hi all,
On Tue, 12 Feb 2019 10:03:09 -0600 Rob Herring robh+dt@kernel.org wrote:
Yeah, that is in Andrew's "post linux-next" patch series, so if you rebase it on top of linux-next and then send it to Andrew with some explanation.
...
Actually, if it is intended for the stable trees, then presumably it is intended to go to Linus for the current release? In which case, the patch in Andrew's tree will have to be changed to cope after your patch appears in Linus' tree (and therefore, linux-next).
On Wed, 13 Feb 2019 08:50:28 +1100 Stephen Rothwell sfr@canb.auug.org.au wrote:
Yup, just do whatever needs to be done and I'll figure it out ;)
On Tue, Feb 12, 2019 at 3:50 PM Stephen Rothwell sfr@canb.auug.org.au wrote:
At this point in the cycle, I wasn't planning to send this for 5.0. It's not fixing something introduced in 5.0 and it is a debug feature.
Rob
On Tue, Feb 12, 2019 at 04:12:24PM -0600, Rob Herring wrote:
Below is the version vs. current mmotm.
From 9ea6dceb46067d4f1cbbdbec1189c8496aa0a4bc Mon Sep 17 00:00:00 2001
From: Mike Rapoport rppt@linux.ibm.com Date: Mon, 4 Feb 2019 15:37:21 +0100 Subject: [PATCH] of: fix kmemleak crash caused by imbalance in early memory reservation
Marc Gonzalez reported the following kmemleak crash:
Unable to handle kernel paging request at virtual address ffffffc021e00000 Mem abort info: ESR = 0x96000006 Exception class = DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 Data abort info: ISV = 0, ISS = 0x00000006 CM = 0, WnR = 0 swapper pgtable: 4k pages, 39-bit VAs, pgdp = (____ptrval____) [ffffffc021e00000] pgd=000000017e3ba803, pud=000000017e3ba803, pmd=0000000000000000 Internal error: Oops: 96000006 [#1] PREEMPT SMP Modules linked in: CPU: 6 PID: 523 Comm: kmemleak Tainted: G S W 5.0.0-rc1 #13 Hardware name: Qualcomm Technologies, Inc. MSM8998 v1 MTP (DT) pstate: 80000085 (Nzcv daIf -PAN -UAO) pc : scan_block+0x70/0x190 lr : scan_block+0x6c/0x190 sp : ffffff8012e8bd20 x29: ffffff8012e8bd20 x28: ffffffc0fdbaf018 x27: ffffffc022000000 x26: 0000000000000080 x25: ffffff8011aadf70 x24: ffffffc0f8cc8000 x23: ffffff8010dc8000 x22: ffffff8010dc8830 x21: ffffffc021e00ff9 x20: ffffffc0f8cc8050 x19: ffffffc021e00000 x18: 0000000000002409 x17: 0000000000000200 x16: 0000000000000000 x15: ffffff8010e14dd8 x14: 0000000000002406 x13: 000000004c4dd0c6 x12: ffffffc0f77dad58 x11: 0000000000000001 x10: ffffff8010d9e688 x9 : ffffff8010d9f000 x8 : ffffff8010d9e688 x7 : 0000000000000002 x6 : 0000000000000000 x5 : ffffff8011511c20 x4 : 00000000000026d1 x3 : ffffff8010e14d88 x2 : 5b36396f4e7d4000 x1 : 0000000000208040 x0 : 0000000000000000 Process kmemleak (pid: 523, stack limit = 0x(____ptrval____)) Call trace: scan_block+0x70/0x190 scan_gray_list+0x108/0x1c0 kmemleak_scan+0x33c/0x7c0 kmemleak_scan_thread+0x98/0xf0 kthread+0x11c/0x120 ret_from_fork+0x10/0x1c Code: f9000fb4 d503201f 97ffffd2 35000580 (f9400260) ---[ end trace 176d6ed9d86a0c33 ]--- note: kmemleak[523] exited with preempt_count 2
The crash happens when a no-map area is allocated in early_init_dt_alloc_reserved_memory_arch(). The allocated region is registered with kmemleak, but it is then removed from memblock using memblock_remove() that is not kmemleak-aware.
Replacing __memblock_alloc_base() with memblock_find_in_range() makes sure that the allocated memory is not added to kmemleak and then memblock_remove()'ing this memory is safe.
As a bonus, since memblock_find_in_range() ensures the allocation in the specified range, the bounds check can be removed.
Cc: stable@vger.kernel.org # 3.15+ Fixes: 3f0c820664483 ("drivers: of: add initialization code for dynamic reserved memory") Acked-by: Marek Szyprowski m.szyprowski@samsung.com Acked-by: Prateek Patel prpatel@nvidia.com Tested-by: Marc Gonzalez marc.w.gonzalez@free.fr Signed-off-by: Mike Rapoport rppt@linux.ibm.com --- drivers/of/of_reserved_mem.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 78aa9eb..47971ab 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -34,21 +34,16 @@ int __init __weak early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
end = !end ? MEMBLOCK_ALLOC_ANYWHERE : end; align = !align ? SMP_CACHE_BYTES : align; - base = memblock_phys_alloc_range(size, align, 0, end); + base = memblock_find_in_range(size, align, start, end); if (!base) return -ENOMEM;
- /* - * Check if the allocated region fits in to start..end window - */ - if (base < start) { - memblock_free(base, size); - return -ENOMEM; - } - *res_base = base; if (nomap) return memblock_remove(base, size); + else + return memblock_reserve(base, size); + return 0; }
On Tue, Feb 12, 2019 at 04:12:24PM -0600, Rob Herring wrote:
Hi Rob,
this may be a debug feature, but we do test our kernels with it enabled, and the problem does affect our 4.19 branch (chromeos-4.19). Are you suggesting that we should backport the fix into our branch and not send the backport to -stable ?
No problem, just trying to avoid wasting our time.
Thanks, Guenter
On Tue, Mar 5, 2019 at 8:12 PM Guenter Roeck linux@roeck-us.net wrote:
No, not at all. Just that I wasn't going to add it to the probable last 5.0-rc and would wait.
However, it's complicated by other memblock changes in 5.1 and not a trivial backport. One of the versions on the list should be easier to backport than what's in mainline (or going to be).
Rob
linux-stable-mirror@lists.linaro.org
-
Andrew Morton -
Guenter Roeck -
Marc Gonzalez -
Mike Rapoport -
Rob Herring -
Stephen Rothwell