diff mbox

devm_memremap_release: fix memremap'd addr handling

Message ID 1455640227-21459-1-git-send-email-toshi.kani@hpe.com (mailing list archive)
State Accepted
Commit 9273a8bbf58a
Headers show

Commit Message

Kani, Toshi Feb. 16, 2016, 4:30 p.m. UTC
The pmem driver calls devm_memremap() to map a persistent memory
range.  When the pmem driver is unloaded, this memremap'd range
is not released.

Fix devm_memremap_release() to handle a given memremap'd address
properly.

Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 kernel/memremap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andrew Morton Feb. 17, 2016, 12:18 a.m. UTC | #1
On Tue, 16 Feb 2016 09:30:27 -0700 Toshi Kani <toshi.kani@hpe.com> wrote:

> The pmem driver calls devm_memremap() to map a persistent memory
> range.  When the pmem driver is unloaded, this memremap'd range
> is not released.
> 
> Fix devm_memremap_release() to handle a given memremap'd address
> properly.
> 
> ...
>
> --- a/kernel/memremap.c
> +++ b/kernel/memremap.c
> @@ -114,7 +114,7 @@ EXPORT_SYMBOL(memunmap);
>  
>  static void devm_memremap_release(struct device *dev, void *res)
>  {
> -	memunmap(res);
> +	memunmap(*(void **)res);
>  }
>  

Huh.  So what happens?  memunmap() decides it isn't a vmalloc address
and we leak a vma?

I'll add a cc:stable to this.
Dan Williams Feb. 17, 2016, 12:40 a.m. UTC | #2
On Tue, Feb 16, 2016 at 8:30 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> The pmem driver calls devm_memremap() to map a persistent memory
> range.  When the pmem driver is unloaded, this memremap'd range
> is not released.
>
> Fix devm_memremap_release() to handle a given memremap'd address
> properly.
>
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
>  kernel/memremap.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/memremap.c b/kernel/memremap.c
> index 2c468de..7a1b5c3 100644
> --- a/kernel/memremap.c
> +++ b/kernel/memremap.c
> @@ -114,7 +114,7 @@ EXPORT_SYMBOL(memunmap);
>
>  static void devm_memremap_release(struct device *dev, void *res)
>  {
> -       memunmap(res);
> +       memunmap(*(void **)res);
>  }

Ugh, yup.  Thanks Toshi!

Acked-by: Dan Williams <dan.j.williams@intel.com>
Kani, Toshi Feb. 17, 2016, 1:16 a.m. UTC | #3
On Tue, 2016-02-16 at 16:18 -0800, Andrew Morton wrote:
> On Tue, 16 Feb 2016 09:30:27 -0700 Toshi Kani <toshi.kani@hpe.com> wrote:
> 
> > The pmem driver calls devm_memremap() to map a persistent memory
> > range.  When the pmem driver is unloaded, this memremap'd range
> > is not released.
> > 
> > Fix devm_memremap_release() to handle a given memremap'd address
> > properly.
> > 
> > ...
> > 
> > --- a/kernel/memremap.c
> > +++ b/kernel/memremap.c
> > @@ -114,7 +114,7 @@ EXPORT_SYMBOL(memunmap);
> >  
> >  static void devm_memremap_release(struct device *dev, void *res)
> >  {
> > -	memunmap(res);
> > +	memunmap(*(void **)res);
> >  }
> >  
> 
> Huh.  So what happens?  memunmap() decides it isn't a vmalloc address
> and we leak a vma?

Yes, that's right.

> I'll add a cc:stable to this.

Agreed.

Thanks!
-Toshi
diff mbox

Patch

diff --git a/kernel/memremap.c b/kernel/memremap.c
index 2c468de..7a1b5c3 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -114,7 +114,7 @@  EXPORT_SYMBOL(memunmap);
 
 static void devm_memremap_release(struct device *dev, void *res)
 {
-	memunmap(res);
+	memunmap(*(void **)res);
 }
 
 static int devm_memremap_match(struct device *dev, void *res, void *match_data)