* [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 related [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.