From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lb0-f199.google.com (mail-lb0-f199.google.com [209.85.217.199]) by kanga.kvack.org (Postfix) with ESMTP id 3E83D6B026E for ; Wed, 20 Apr 2016 06:53:43 -0400 (EDT) Received: by mail-lb0-f199.google.com with SMTP id wy10so7888805lbb.3 for ; Wed, 20 Apr 2016 03:53:43 -0700 (PDT) Received: from fnsib-smtp07.srv.cat (fnsib-smtp07.srv.cat. [46.16.61.67]) by mx.google.com with ESMTPS id w2si9372091wma.29.2016.04.20.03.53.41 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 Apr 2016 03:53:41 -0700 (PDT) Received: from vostok.local (cliente152.wlan.uam.es [150.244.199.152]) by fnsib-smtp07.srv.cat (Postfix) with ESMTPA id 3FC34811F for ; Wed, 20 Apr 2016 12:53:39 +0200 (CEST) Date: Wed, 20 Apr 2016 12:53:33 +0200 From: =?utf-8?Q?Guillermo_Juli=C3=A1n_Moreno?= Message-ID: Subject: [PATCH] mm: fix overflow in vm_map_ram MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Sender: owner-linux-mm@kvack.org List-ID: To: linux-mm@kvack.org When remapping pages accounting for 4G or more memory space, the operation 'count << PAGE=5FSHI=46T' overflows as it is performed on an integer. Solution: cast before doing the bitshift. Signed-off-by: Guillermo Juli=C3=A1n --- mm/vmalloc.c =7C 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/vmalloc.c b/mm/vmalloc.c =20 index ae7d20b..97257e4 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c =40=40 -1114,7 +1114,7 =40=40 EXPORT=5FSYMBOL(vm=5Funmap=5Fram); */ void *vm=5Fmap=5Fram(struct page **pages, unsigned int count, int node, p= gprot=5Ft prot) =7B - unsigned long size =3D count << PAGE=5FSHI=46T; + unsigned long size =3D ((unsigned long) count) << PAGE=5FSHI=46T; unsigned long addr; void *mem; =40=40 -1484,7 +1484,7 =40=40 static void =5F=5Fvunmap(const void *addr, = int deallocate=5Fpages) =20 kfree(area); return; =7D - + /** * vfree - release memory allocated by vmalloc() * =40addr: 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f70.google.com (mail-wm0-f70.google.com [74.125.82.70]) by kanga.kvack.org (Postfix) with ESMTP id 076216B007E for ; Thu, 26 May 2016 03:38:17 -0400 (EDT) Received: by mail-wm0-f70.google.com with SMTP id n2so40783795wma.0 for ; Thu, 26 May 2016 00:38:16 -0700 (PDT) Received: from fnsib-smtp05.srv.cat (fnsib-smtp05.srv.cat. [46.16.61.54]) by mx.google.com with ESMTPS id r19si7105980lfe.307.2016.05.26.00.38.14 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 26 May 2016 00:38:14 -0700 (PDT) Received: from vostok.local.mail (boro.ii.uam.es [150.244.58.71]) by fnsib-smtp05.srv.cat (Postfix) with ESMTPA id 719431EF111 for ; Thu, 26 May 2016 09:38:12 +0200 (CEST) Date: Thu, 26 May 2016 09:38:04 +0200 From: =?utf-8?Q?Guillermo_Juli=C3=A1n_Moreno?= Message-ID: In-Reply-To: References: Subject: Re: [PATCH] mm: fix overflow in vm_map_ram MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Sender: owner-linux-mm@kvack.org List-ID: To: linux-mm@kvack.org On 20 April 2016 at 12:53:41, Guillermo Juli=C3=A1n Moreno (guillermo.jul= ian=40naudit.es(mailto:guillermo.julian=40naudit.es)) wrote: > =20 > When remapping pages accounting for 4G or more memory space, the > operation 'count << PAGE=5FSHI=46T' overflows as it is performed on an > integer. Solution: cast before doing the bitshift. > =20 > Signed-off-by: Guillermo Juli=C3=A1n =20 > --- > mm/vmalloc.c =7C 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > =20 > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index ae7d20b..97257e4 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > =40=40 -1114,7 +1114,7 =40=40 EXPORT=5FSYMBOL(vm=5Funmap=5Fram); > */ > void *vm=5Fmap=5Fram(struct page **pages, unsigned int count, int node,= pgprot=5Ft prot) > =7B > - unsigned long size =3D count << PAGE=5FSHI=46T; > + unsigned long size =3D ((unsigned long) count) << PAGE=5FSHI=46T; > unsigned long addr; > void *mem; > =20 > =40=40 -1484,7 +1484,7 =40=40 static void =5F=5Fvunmap(const void *addr= , int deallocate=5Fpages) > kfree(area); > return; > =7D > - > + > /** > * vfree - release memory allocated by vmalloc() > * =40addr: memory base address > -- > 1.8.3.1 Hello, has anyone taken a look at this patch=3F Guillermo Juli=C3=A1n -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f199.google.com (mail-pf0-f199.google.com [209.85.192.199]) by kanga.kvack.org (Postfix) with ESMTP id 343FA6B025E for ; Thu, 26 May 2016 17:28:39 -0400 (EDT) Received: by mail-pf0-f199.google.com with SMTP id c84so61615100pfc.3 for ; Thu, 26 May 2016 14:28:39 -0700 (PDT) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org. [140.211.169.12]) by mx.google.com with ESMTPS id sr4si23150263pab.10.2016.05.26.14.28.38 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 26 May 2016 14:28:38 -0700 (PDT) Date: Thu, 26 May 2016 14:28:37 -0700 From: Andrew Morton Subject: Re: [PATCH] mm: fix overflow in vm_map_ram Message-Id: <20160526142837.662100b01ff094be9a28f01b@linux-foundation.org> In-Reply-To: References: Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Guillermo =?ISO-8859-1?Q?Juli=E1n?= Moreno Cc: linux-mm@kvack.org On Wed, 20 Apr 2016 12:53:33 +0200 Guillermo Juli__n Moreno 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 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 Signed-off-by: Andrew Morton --- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f71.google.com (mail-lf0-f71.google.com [209.85.215.71]) by kanga.kvack.org (Postfix) with ESMTP id E6C1D6B007E for ; Fri, 27 May 2016 04:26:02 -0400 (EDT) Received: by mail-lf0-f71.google.com with SMTP id w16so43304692lfd.0 for ; Fri, 27 May 2016 01:26:02 -0700 (PDT) Received: from fnsib-smtp01.srv.cat (fnsib-smtp01.srv.cat. [46.16.60.186]) by mx.google.com with ESMTPS id x124si8153386lfd.231.2016.05.27.01.26.01 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 27 May 2016 01:26:01 -0700 (PDT) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Fri, 27 May 2016 10:25:59 +0200 From: "guillermo.julian" Subject: Re: [PATCH] mm: fix overflow in vm_map_ram Reply-To: guillermo.julian@naudit.es In-Reply-To: <20160526142837.662100b01ff094be9a28f01b@linux-foundation.org> References: <20160526142837.662100b01ff094be9a28f01b@linux-foundation.org> Message-ID: <08d280dc9c9fe037805e3ff74d7dad02@naudit.es> Sender: owner-linux-mm@kvack.org List-ID: To: Andrew Morton Cc: linux-mm@kvack.org El 2016-05-26 23:28, Andrew Morton escribiA3: > On Wed, 20 Apr 2016 12:53:33 +0200 Guillermo Juli__n Moreno > 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 > 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 > Signed-off-by: Andrew Morton > --- > > 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f70.google.com (mail-pa0-f70.google.com [209.85.220.70]) by kanga.kvack.org (Postfix) with ESMTP id 478AA6B007E for ; Fri, 27 May 2016 16:20:37 -0400 (EDT) Received: by mail-pa0-f70.google.com with SMTP id di3so55437102pab.0 for ; Fri, 27 May 2016 13:20:37 -0700 (PDT) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org. [140.211.169.12]) by mx.google.com with ESMTPS id sk6si30479838pab.145.2016.05.27.13.20.36 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 27 May 2016 13:20:36 -0700 (PDT) Date: Fri, 27 May 2016 13:20:35 -0700 From: Andrew Morton Subject: Re: [PATCH] mm: fix overflow in vm_map_ram Message-Id: <20160527132035.0239af56b4887e89e7c3b962@linux-foundation.org> In-Reply-To: <08d280dc9c9fe037805e3ff74d7dad02@naudit.es> References: <20160526142837.662100b01ff094be9a28f01b@linux-foundation.org> <08d280dc9c9fe037805e3ff74d7dad02@naudit.es> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: guillermo.julian@naudit.es Cc: linux-mm@kvack.org On Fri, 27 May 2016 10:25:59 +0200 "guillermo.julian" wrote: > El 2016-05-26 23:28, Andrew Morton escribi__: > > On Wed, 20 Apr 2016 12:53:33 +0200 Guillermo Juli__n Moreno > > 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 Subject: mm-fix-overflow-in-vm_map_ram-fix fix vmap() as well, per Guillermo Cc: Guillermo Juli_n Moreno Signed-off-by: Andrew Morton --- 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: email@kvack.org