All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm: fix overflow in vm_map_ram
@ 2016-04-20 10:53 Guillermo Julián Moreno
  2016-05-26  7:38 ` Guillermo Julián Moreno
  2016-05-26 21:28 ` Andrew Morton
  0 siblings, 2 replies; 5+ messages in thread
From: Guillermo Julián Moreno @ 2016-04-20 10:53 UTC (permalink / raw)
  To: linux-mm


When remapping pages accounting for 4G or more memory space, the
operation 'count << PAGE_SHIFT' overflows as it is performed on an
integer. Solution: cast before doing the bitshift.

Signed-off-by: Guillermo Julián <guillermo.julian@naudit.es>
---
mm/vmalloc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c  
index ae7d20b..97257e4 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1114,7 +1114,7 @@ EXPORT_SYMBOL(vm_unmap_ram);
*/
void *vm_map_ram(struct page **pages, unsigned int count, int node, pgprot_t prot)
{
- unsigned long size = count << PAGE_SHIFT;
+ unsigned long size = ((unsigned long) count) << PAGE_SHIFT;
unsigned long addr;
void *mem;

@@ -1484,7 +1484,7 @@ static void __vunmap(const void *addr, int deallocate_pages)  
kfree(area);
return;
}
-
+
/**
* vfree - release memory allocated by vmalloc()
* @addr: memory base address
--
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm: fix overflow in vm_map_ram
  2016-04-20 10:53 [PATCH] mm: fix overflow in vm_map_ram Guillermo Julián Moreno
@ 2016-05-26  7:38 ` Guillermo Julián Moreno
  2016-05-26 21:28 ` Andrew Morton
  1 sibling, 0 replies; 5+ messages in thread
From: Guillermo Julián Moreno @ 2016-05-26  7:38 UTC (permalink / raw)
  To: linux-mm


On 20 April 2016 at 12:53:41, Guillermo Julián Moreno (guillermo.julian@naudit.es(mailto:guillermo.julian@naudit.es)) wrote:

>  
> When remapping pages accounting for 4G or more memory space, the
> operation 'count << PAGE_SHIFT' overflows as it is performed on an
> integer. Solution: cast before doing the bitshift.
>  
> Signed-off-by: Guillermo Julián  
> ---
> mm/vmalloc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>  
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index ae7d20b..97257e4 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1114,7 +1114,7 @@ EXPORT_SYMBOL(vm_unmap_ram);
> */
> void *vm_map_ram(struct page **pages, unsigned int count, int node, pgprot_t prot)
> {
> - unsigned long size = count << PAGE_SHIFT;
> + unsigned long size = ((unsigned long) count) << PAGE_SHIFT;
> unsigned long addr;
> void *mem;
>  
> @@ -1484,7 +1484,7 @@ static void __vunmap(const void *addr, int deallocate_pages)
> kfree(area);
> return;
> }
> -
> +
> /**
> * vfree - release memory allocated by vmalloc()
> * @addr: memory base address
> --
> 1.8.3.1

Hello, has anyone taken a look at this patch?

Guillermo Julián



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm: fix overflow in vm_map_ram
  2016-04-20 10:53 [PATCH] mm: fix overflow in vm_map_ram Guillermo Julián Moreno
  2016-05-26  7:38 ` Guillermo Julián Moreno
@ 2016-05-26 21:28 ` Andrew Morton
  2016-05-27  8:25   ` guillermo.julian
  1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2016-05-26 21:28 UTC (permalink / raw)
  To: Guillermo Julián Moreno; +Cc: linux-mm

On Wed, 20 Apr 2016 12:53:33 +0200 Guillermo Juli__n Moreno <guillermo.julian@naudit.es> wrote:

> 
> When remapping pages accounting for 4G or more memory space, the
> operation 'count << PAGE_SHIFT' overflows as it is performed on an
> integer. Solution: cast before doing the bitshift.

Yup.

We need to work out which kernel versions to fix.  What are the runtime
effects of this?  Are there real drivers in the tree which actually map
more than 4G?

I fixed vm_unmap_ram() as well, but I didn't test it.  I wonder why you
missed that...

> diff --git a/mm/vmalloc.c b/mm/vmalloc.c  
> index ae7d20b..97257e4 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1114,7 +1114,7 @@ EXPORT_SYMBOL(vm_unmap_ram);
> */
> void *vm_map_ram(struct page **pages, unsigned int count, int node, pgprot_t prot)
> {
> - unsigned long size = count << PAGE_SHIFT;
> + unsigned long size = ((unsigned long) count) << PAGE_SHIFT;
> unsigned long addr;
> void *mem;
> 

Your email client totally messes up the patches.  Please fix that for
next time.


From: Guillermo Juli_n Moreno <guillermo.julian@naudit.es>
Subject: mm: fix overflow in vm_map_ram()

When remapping pages accounting for 4G or more memory space, the
operation 'count << PAGE_SHIFT' overflows as it is performed on an
integer. Solution: cast before doing the bitshift.

[akpm@linux-foundation.org: fix vm_unmap_ram() also]
Link: http://lkml.kernel.org/r/etPan.57175fb3.7a271c6b.2bd@naudit.es
Signed-off-by: Guillermo Juli_n Moreno <guillermo.julian@naudit.es>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/vmalloc.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -puN mm/vmalloc.c~mm-fix-overflow-in-vm_map_ram mm/vmalloc.c
--- a/mm/vmalloc.c~mm-fix-overflow-in-vm_map_ram
+++ a/mm/vmalloc.c
@@ -1105,7 +1105,7 @@ EXPORT_SYMBOL_GPL(vm_unmap_aliases);
  */
 void vm_unmap_ram(const void *mem, unsigned int count)
 {
-	unsigned long size = count << PAGE_SHIFT;
+	unsigned long size = (unsigned long)count << PAGE_SHIFT;
 	unsigned long addr = (unsigned long)mem;
 
 	BUG_ON(!addr);
@@ -1140,7 +1140,7 @@ EXPORT_SYMBOL(vm_unmap_ram);
  */
 void *vm_map_ram(struct page **pages, unsigned int count, int node, pgprot_t prot)
 {
-	unsigned long size = count << PAGE_SHIFT;
+	unsigned long size = (unsigned long)count << PAGE_SHIFT;
 	unsigned long addr;
 	void *mem;
 
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm: fix overflow in vm_map_ram
  2016-05-26 21:28 ` Andrew Morton
@ 2016-05-27  8:25   ` guillermo.julian
  2016-05-27 20:20     ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: guillermo.julian @ 2016-05-27  8:25 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-mm

El 2016-05-26 23:28, Andrew Morton escribiA3:
> On Wed, 20 Apr 2016 12:53:33 +0200 Guillermo Juli__n Moreno
> <guillermo.julian@naudit.es> wrote:
> 
>> 
>> When remapping pages accounting for 4G or more memory space, the
>> operation 'count << PAGE_SHIFT' overflows as it is performed on an
>> integer. Solution: cast before doing the bitshift.
> 
> Yup.
> 
> We need to work out which kernel versions to fix.  What are the runtime
> effects of this?  Are there real drivers in the tree which actually map
> more than 4G?

Looking at the references of vm_map_ram, it is only used in three 
drivers (XFS, v4l2-core and android/ion). However, in the vmap() code, 
the same bug is likely to occur (vmalloc.c:1557), and that function is 
more frequently used. But if it has gone unnoticed until now, most 
probably it isn't a critical issue (4G memory allocations are usually 
not needed. In fact this bug surfaced during a performance test in a 
modified driver, not in a regular configuration.

> 
> I fixed vm_unmap_ram() as well, but I didn't test it.  I wonder why you
> missed that...

The initial test didn't fail so I didn't notice the unmap was not 
working, so I completely forgot to check that function.

> 
>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>> index ae7d20b..97257e4 100644
>> --- a/mm/vmalloc.c
>> +++ b/mm/vmalloc.c
>> @@ -1114,7 +1114,7 @@ EXPORT_SYMBOL(vm_unmap_ram);
>> */
>> void *vm_map_ram(struct page **pages, unsigned int count, int node, 
>> pgprot_t prot)
>> {
>> - unsigned long size = count << PAGE_SHIFT;
>> + unsigned long size = ((unsigned long) count) << PAGE_SHIFT;
>> unsigned long addr;
>> void *mem;
>> 
> 
> Your email client totally messes up the patches.  Please fix that for
> next time.

Sorry about that, I didn't notice it ate the tabs. I checked and this 
time it shouldn't happen.

> 
> 
> From: Guillermo Juli_n Moreno <guillermo.julian@naudit.es>
> Subject: mm: fix overflow in vm_map_ram()
> 
> When remapping pages accounting for 4G or more memory space, the
> operation 'count << PAGE_SHIFT' overflows as it is performed on an
> integer. Solution: cast before doing the bitshift.
> 
> [akpm@linux-foundation.org: fix vm_unmap_ram() also]
> Link: http://lkml.kernel.org/r/etPan.57175fb3.7a271c6b.2bd@naudit.es
> Signed-off-by: Guillermo Juli_n Moreno <guillermo.julian@naudit.es>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  mm/vmalloc.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff -puN mm/vmalloc.c~mm-fix-overflow-in-vm_map_ram mm/vmalloc.c
> --- a/mm/vmalloc.c~mm-fix-overflow-in-vm_map_ram
> +++ a/mm/vmalloc.c
> @@ -1105,7 +1105,7 @@ EXPORT_SYMBOL_GPL(vm_unmap_aliases);
>   */
>  void vm_unmap_ram(const void *mem, unsigned int count)
>  {
> -	unsigned long size = count << PAGE_SHIFT;
> +	unsigned long size = (unsigned long)count << PAGE_SHIFT;
>  	unsigned long addr = (unsigned long)mem;
> 
>  	BUG_ON(!addr);
> @@ -1140,7 +1140,7 @@ EXPORT_SYMBOL(vm_unmap_ram);
>   */
>  void *vm_map_ram(struct page **pages, unsigned int count, int node,
> pgprot_t prot)
>  {
> -	unsigned long size = count << PAGE_SHIFT;
> +	unsigned long size = (unsigned long)count << PAGE_SHIFT;
>  	unsigned long addr;
>  	void *mem;
> 
> _

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm: fix overflow in vm_map_ram
  2016-05-27  8:25   ` guillermo.julian
@ 2016-05-27 20:20     ` Andrew Morton
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2016-05-27 20:20 UTC (permalink / raw)
  To: guillermo.julian; +Cc: linux-mm

On Fri, 27 May 2016 10:25:59 +0200 "guillermo.julian" <guillermo.julian@naudit.es> wrote:

> El 2016-05-26 23:28, Andrew Morton escribi__:
> > On Wed, 20 Apr 2016 12:53:33 +0200 Guillermo Juli__n Moreno
> > <guillermo.julian@naudit.es> wrote:
> > 
> >> 
> >> When remapping pages accounting for 4G or more memory space, the
> >> operation 'count << PAGE_SHIFT' overflows as it is performed on an
> >> integer. Solution: cast before doing the bitshift.
> > 
> > Yup.
> > 
> > We need to work out which kernel versions to fix.  What are the runtime
> > effects of this?  Are there real drivers in the tree which actually map
> > more than 4G?
> 
> Looking at the references of vm_map_ram, it is only used in three 
> drivers (XFS, v4l2-core and android/ion). However, in the vmap() code, 
> the same bug is likely to occur (vmalloc.c:1557), and that function is 
> more frequently used. But if it has gone unnoticed until now, most 
> probably it isn't a critical issue (4G memory allocations are usually 
> not needed. In fact this bug surfaced during a performance test in a 
> modified driver, not in a regular configuration.

Yup.  I'll add this as well:

From: Andrew Morton <akpm@linux-foundation.org>
Subject: mm-fix-overflow-in-vm_map_ram-fix

fix vmap() as well, per Guillermo

Cc: Guillermo Juli_n Moreno <guillermo.julian@naudit.es>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/vmalloc.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff -puN mm/vmalloc.c~mm-fix-overflow-in-vm_map_ram-fix mm/vmalloc.c
--- a/mm/vmalloc.c~mm-fix-overflow-in-vm_map_ram-fix
+++ a/mm/vmalloc.c
@@ -1574,14 +1574,15 @@ void *vmap(struct page **pages, unsigned
 		unsigned long flags, pgprot_t prot)
 {
 	struct vm_struct *area;
+	unsigned long size;		/* In bytes */
 
 	might_sleep();
 
 	if (count > totalram_pages)
 		return NULL;
 
-	area = get_vm_area_caller((count << PAGE_SHIFT), flags,
-					__builtin_return_address(0));
+	size = (unsigned long)count << PAGE_SHIFT;
+	area = get_vm_area_caller(size, flags, __builtin_return_address(0));
 	if (!area)
 		return NULL;
 
_


I checked all other instances of "<< PAGE" in vmalloc.c and we're good.
Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-05-27 20:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-20 10:53 [PATCH] mm: fix overflow in vm_map_ram Guillermo Julián Moreno
2016-05-26  7:38 ` Guillermo Julián Moreno
2016-05-26 21:28 ` Andrew Morton
2016-05-27  8:25   ` guillermo.julian
2016-05-27 20:20     ` Andrew Morton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.