Re: [PATCH v2] of: fix kmemleak crash

Re: [PATCH v2] of: fix kmemleak crash

[Marc Gonzalez]

+ GKH

On 01/02/2019 17:23, Marc Gonzalez wrote:

> On 23/01/2019 13:31, Mike Rapoport wrote:
> 
>> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
>> Tested-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
>> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  drivers/of/of_reserved_mem.c | 18 +++++-------------
>>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> Thanks for the patch, Mike.
> 
> Whose tree should this patch go through?

By the way, I think we can add

Acked-by: Prateek Patel <prpatel@nvidia.com>
Fixes: 3f0c820664483 ("drivers: of: add initialization code for dynamic reserved memory")
Cc: stable@vger.kernel.org # 3.15+

Regards.

Re: [PATCH v2] of: fix kmemleak crash

[Greg Kroah-Hartman]

On Mon, Feb 04, 2019 at 11:10:39AM +0100, Marc Gonzalez wrote:
> + GKH
> 
> On 01/02/2019 17:23, Marc Gonzalez wrote:
> 
> > On 23/01/2019 13:31, Mike Rapoport wrote:
> > 
> >> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> >> Tested-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> >> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> ---
> >>  drivers/of/of_reserved_mem.c | 18 +++++-------------
> >>  1 file changed, 5 insertions(+), 13 deletions(-)
> > 
> > Thanks for the patch, Mike.
> > 
> > Whose tree should this patch go through?
> 
> By the way, I think we can add
> 
> Acked-by: Prateek Patel <prpatel@nvidia.com>
> Fixes: 3f0c820664483 ("drivers: of: add initialization code for dynamic reserved memory")
> Cc: stable@vger.kernel.org # 3.15+

What am I supposed to do with this?

confused,

greg k-h

Re: [PATCH v2] of: fix kmemleak crash

[Rob Herring]

On Mon, Feb 4, 2019 at 9:25 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Feb 04, 2019 at 11:10:39AM +0100, Marc Gonzalez wrote:
> > + GKH
> >
> > On 01/02/2019 17:23, Marc Gonzalez wrote:
> >
> > > On 23/01/2019 13:31, Mike Rapoport wrote:
> > >
> > >> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> > >> Tested-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> > >> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > >> ---
> > >>  drivers/of/of_reserved_mem.c | 18 +++++-------------
> > >>  1 file changed, 5 insertions(+), 13 deletions(-)
> > >
> > > Thanks for the patch, Mike.
> > >
> > > Whose tree should this patch go through?
> >
> > By the way, I think we can add
> >
> > Acked-by: Prateek Patel <prpatel@nvidia.com>
> > Fixes: 3f0c820664483 ("drivers: of: add initialization code for dynamic reserved memory")
> > Cc: stable@vger.kernel.org # 3.15+
>
> What am I supposed to do with this?

Nothing til I apply it.

Rob

[RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Marc Gonzalez]

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;
 }
 
-- 
2.7.4

Re: [RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Mike Rapoport]

Hi,

On Thu, Feb 07, 2019 at 04:32:13PM +0000, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: 3f0c82066448 drivers: of: add initialization code for dynamic reserved memory.
> 
> The bot has tested the following trees: v4.20.6, v4.19.19, v4.14.97, v4.9.154, v4.4.172, v3.18.133.

...

> How should we proceed with this patch?

Below is the backport of the patch to 4.19.19 and below. I've build-tested
for 4.19 and verified it applies cleanly to the older versions.

From 2052bf06134fd8feae3e621acea6421f916bbbae 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 | 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 895c83e..240763b 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -32,26 +32,18 @@ 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;
-	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;
 }
 #else
-- 
2.7.4


> --
> Thanks,
> Sasha
> 

-- 
Sincerely yours,
Mike.

Re: [RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Marc Gonzalez]

On 04/02/2019 15:37, Marc Gonzalez wrote:

> 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

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.

Re: [RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Rob Herring]

On Mon, Feb 11, 2019 at 10:47 AM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
>
> On 04/02/2019 15:37, Marc Gonzalez wrote:
>
> > 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
>
> 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?

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

Re: [RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

Hi all,

On Tue, 12 Feb 2019 10:03:09 -0600 Rob Herring <robh+dt@kernel.org> wrote:
>
> On Mon, Feb 11, 2019 at 10:47 AM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
> >
> > On 04/02/2019 15:37, Marc Gonzalez wrote:
> >  
> > > 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  
> >
> > 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?  
> 
> 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.

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).

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Andrew Morton]

On Wed, 13 Feb 2019 08:50:28 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> > > It's definitely going to conflict with the proposed patch
> > > over drivers/of/of_reserved_mem.c
> > >
> > > Rob, what's the next step then?  
> > 
> > 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.
> 
> 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).

Yup, just do whatever needs to be done and I'll figure it out ;)

Re: [RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Rob Herring]

On Tue, Feb 12, 2019 at 3:50 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> On Tue, 12 Feb 2019 10:03:09 -0600 Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Mon, Feb 11, 2019 at 10:47 AM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
> > >
> > > On 04/02/2019 15:37, Marc Gonzalez wrote:
> > >
> > > > 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
> > >
> > > 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?
> >
> > 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.
>
> 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).

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

Re: [RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Mike Rapoport]

On Tue, Feb 12, 2019 at 04:12:24PM -0600, Rob Herring wrote:
> On Tue, Feb 12, 2019 at 3:50 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Hi all,
> >
> > On Tue, 12 Feb 2019 10:03:09 -0600 Rob Herring <robh+dt@kernel.org> wrote:
> > >
> > > On Mon, Feb 11, 2019 at 10:47 AM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
> > > >
> > > > On 04/02/2019 15:37, Marc Gonzalez wrote:
> > > >
> > > > > 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
> > > >
> > > > 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?
> > >
> > > 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.
> >
> > 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).
> 
> 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.
 
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;
 }
 
-- 
2.7.4

Re: [RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Marc Gonzalez]

On 13/02/2019 07:57, Mike Rapoport 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

Out of curiosity, why don't you send as a proper v3?

> Marc Gonzalez reported the following kmemleak crash:
> 
> [...]
> 
> 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()

Nit: in this new version, you're replacing memblock_phys_alloc_range()
with memblock_find_in_range() so I don't know if the comment still
applies.

> 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>

Re: [RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Mike Rapoport]

On Wed, Feb 13, 2019 at 10:27:48AM +0100, Marc Gonzalez wrote:
> On 13/02/2019 07:57, Mike Rapoport 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
> 
> Out of curiosity, why don't you send as a proper v3?
 
Was too much in a hurry

> > Marc Gonzalez reported the following kmemleak crash:
> > 
> > [...]
> > 
> > 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()
> 
> Nit: in this new version, you're replacing memblock_phys_alloc_range()
> with memblock_find_in_range() so I don't know if the comment still
> applies.

and didn't check the outcome of blindly applying the patch :(

I'll send a proper v3 soon. Sorry for the noise.
 
> > 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>
> 

-- 
Sincerely yours,
Mike.

Re: [RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Guenter Roeck]

On Tue, Feb 12, 2019 at 04:12:24PM -0600, Rob Herring wrote:
> On Tue, Feb 12, 2019 at 3:50 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Hi all,
> >
> > On Tue, 12 Feb 2019 10:03:09 -0600 Rob Herring <robh+dt@kernel.org> wrote:
> > >
> > > On Mon, Feb 11, 2019 at 10:47 AM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
> > > >
> > > > On 04/02/2019 15:37, Marc Gonzalez wrote:
> > > >
> > > > > 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
> > > >
> > > > 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?
> > >
> > > 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.
> >
> > 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).
> 
> 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.
> 
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

Re: [RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Rob Herring]

On Tue, Mar 5, 2019 at 8:12 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Tue, Feb 12, 2019 at 04:12:24PM -0600, Rob Herring wrote:
> > On Tue, Feb 12, 2019 at 3:50 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > >
> > > Hi all,
> > >
> > > On Tue, 12 Feb 2019 10:03:09 -0600 Rob Herring <robh+dt@kernel.org> wrote:
> > > >
> > > > On Mon, Feb 11, 2019 at 10:47 AM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
> > > > >
> > > > > On 04/02/2019 15:37, Marc Gonzalez wrote:
> > > > >
> > > > > > 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
> > > > >
> > > > > 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?
> > > >
> > > > 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.
> > >
> > > 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).
> >
> > 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.
> >
> 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, 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

Re: [RESEND PATCH v2] of: fix kmemleak crash caused by imbalance in early memory reservation

[Guenter Roeck]

On 3/6/19 5:39 AM, Rob Herring wrote:
> On Tue, Mar 5, 2019 at 8:12 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On Tue, Feb 12, 2019 at 04:12:24PM -0600, Rob Herring wrote:
>>> On Tue, Feb 12, 2019 at 3:50 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>>>
>>>> Hi all,
>>>>
>>>> On Tue, 12 Feb 2019 10:03:09 -0600 Rob Herring <robh+dt@kernel.org> wrote:
>>>>>
>>>>> On Mon, Feb 11, 2019 at 10:47 AM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
>>>>>>
>>>>>> On 04/02/2019 15:37, Marc Gonzalez wrote:
>>>>>>
>>>>>>> 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
>>>>>>
>>>>>> 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?
>>>>>
>>>>> 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.
>>>>
>>>> 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).
>>>
>>> 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.
>>>
>> 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, 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).
> 

We went ahead and applied a backport of an older version of the patch series
to chromeos-4.19. We'll see how well that works, but so far it looks like
it fixes our problem.

Guenter