* [PATCH] kernel/resource: Fix return code check in __request_free_mem_region
@ 2021-05-12 7:35 Alistair Popple
2021-05-12 12:16 ` David Hildenbrand
0 siblings, 1 reply; 3+ messages in thread
From: Alistair Popple @ 2021-05-12 7:35 UTC (permalink / raw)
To: akpm
Cc: bsingharora, dan.j.williams, daniel.vetter, david, gregkh,
jglisse, jhubbard, linux-mm, smuchun, linux-kernel,
Alistair Popple, kernel test robot
Splitting an earlier version of a patch that allowed calling
__request_region() while holding the resource lock into a series of
patches required changing the return code for the newly introduced
__request_region_locked().
Unfortunately this change was not carried through to a subsequent
commit 56fd94919b8b ("kernel/resource: fix locking in
request_free_mem_region") in the series. This resulted in a
use-after-free due to freeing the struct resource without properly
releasing it. Fix this by correcting the return code check so that the
struct is not freed if the request to add it was successful.
Reported-by: kernel test robot <oliver.sang@intel.com>
Fixes: 56fd94919b8b ("kernel/resource: fix locking in request_free_mem_region")
Signed-off-by: Alistair Popple <apopple@nvidia.com>
---
kernel/resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index 028a5ab18818..ca9f5198a01f 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1805,7 +1805,7 @@ static struct resource *__request_free_mem_region(struct device *dev,
REGION_DISJOINT)
continue;
- if (!__request_region_locked(res, &iomem_resource, addr, size,
+ if (__request_region_locked(res, &iomem_resource, addr, size,
name, 0))
break;
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] kernel/resource: Fix return code check in __request_free_mem_region
2021-05-12 7:35 [PATCH] kernel/resource: Fix return code check in __request_free_mem_region Alistair Popple
@ 2021-05-12 12:16 ` David Hildenbrand
2021-05-12 13:47 ` Alistair Popple
0 siblings, 1 reply; 3+ messages in thread
From: David Hildenbrand @ 2021-05-12 12:16 UTC (permalink / raw)
To: Alistair Popple, akpm
Cc: bsingharora, dan.j.williams, daniel.vetter, gregkh, jglisse,
jhubbard, linux-mm, smuchun, linux-kernel, kernel test robot
On 12.05.21 09:35, Alistair Popple wrote:
> Splitting an earlier version of a patch that allowed calling
> __request_region() while holding the resource lock into a series of
> patches required changing the return code for the newly introduced
> __request_region_locked().
>
> Unfortunately this change was not carried through to a subsequent
> commit 56fd94919b8b ("kernel/resource: fix locking in
> request_free_mem_region") in the series. This resulted in a
> use-after-free due to freeing the struct resource without properly
> releasing it. Fix this by correcting the return code check so that the
> struct is not freed if the request to add it was successful.
>
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Fixes: 56fd94919b8b ("kernel/resource: fix locking in request_free_mem_region")
> Signed-off-by: Alistair Popple <apopple@nvidia.com>
> ---
> kernel/resource.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 028a5ab18818..ca9f5198a01f 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -1805,7 +1805,7 @@ static struct resource *__request_free_mem_region(struct device *dev,
> REGION_DISJOINT)
> continue;
>
> - if (!__request_region_locked(res, &iomem_resource, addr, size,
> + if (__request_region_locked(res, &iomem_resource, addr, size,
> name, 0))
> break;
>
>
Ouch, missed that, would have expected this pops up right away when testing.
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] kernel/resource: Fix return code check in __request_free_mem_region
2021-05-12 12:16 ` David Hildenbrand
@ 2021-05-12 13:47 ` Alistair Popple
0 siblings, 0 replies; 3+ messages in thread
From: Alistair Popple @ 2021-05-12 13:47 UTC (permalink / raw)
To: David Hildenbrand
Cc: akpm, bsingharora, dan.j.williams, daniel.vetter, gregkh,
jglisse, jhubbard, linux-mm, smuchun, linux-kernel,
kernel test robot
On Wednesday, 12 May 2021 10:16:41 PM AEST David Hildenbrand wrote:
> On 12.05.21 09:35, Alistair Popple wrote:
> > Splitting an earlier version of a patch that allowed calling
> > __request_region() while holding the resource lock into a series of
> > patches required changing the return code for the newly introduced
> > __request_region_locked().
> >
> > Unfortunately this change was not carried through to a subsequent
> > commit 56fd94919b8b ("kernel/resource: fix locking in
> > request_free_mem_region") in the series. This resulted in a
> > use-after-free due to freeing the struct resource without properly
> > releasing it. Fix this by correcting the return code check so that the
> > struct is not freed if the request to add it was successful.
> >
> > Reported-by: kernel test robot <oliver.sang@intel.com>
> > Fixes: 56fd94919b8b ("kernel/resource: fix locking in
request_free_mem_region")
> > Signed-off-by: Alistair Popple <apopple@nvidia.com>
> > ---
> > kernel/resource.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/resource.c b/kernel/resource.c
> > index 028a5ab18818..ca9f5198a01f 100644
> > --- a/kernel/resource.c
> > +++ b/kernel/resource.c
> > @@ -1805,7 +1805,7 @@ static struct resource
*__request_free_mem_region(struct device *dev,
> > REGION_DISJOINT)
> > continue;
> >
> > - if (!__request_region_locked(res, &iomem_resource, addr,
size,
> > + if (__request_region_locked(res, &iomem_resource, addr,
size,
> > name, 0))
> > break;
> >
> >
>
> Ouch, missed that, would have expected this pops up right away when testing.
Yes, ouch indeed. I am still trying to figure out why I didn't catch this
right away as well. I retested locally and the HMM tests do complete without
causing an oops although they don't all pass. I guess I must have been in a
rush and didn't check the test output properly to see if they actually passed.
> Reviewed-by: David Hildenbrand <david@redhat.com>
Thanks, and sorry for the extra noise.
> --
> Thanks,
>
> David / dhildenb
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-05-12 13:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-12 7:35 [PATCH] kernel/resource: Fix return code check in __request_free_mem_region Alistair Popple
2021-05-12 12:16 ` David Hildenbrand
2021-05-12 13:47 ` Alistair Popple
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).