* Re: [PATCH] kernel buffer overflow kmalloc_slab() fix
[not found] ` <alpine.DEB.2.00.1105191550001.12530@router.home>
@ 2011-05-20 12:02 ` James Bottomley
2011-05-20 14:33 ` Christoph Lameter
0 siblings, 1 reply; 5+ messages in thread
From: James Bottomley @ 2011-05-20 12:02 UTC (permalink / raw)
To: Christoph Lameter
Cc: james_p_freyensee, linux-mm, gregkh, hari.k.kanigeri, linux-arch
On Thu, 2011-05-19 at 15:51 -0500, Christoph Lameter wrote:
> On Thu, 19 May 2011, james_p_freyensee@linux.intel.com wrote:
>
> > From: J Freyensee <james_p_freyensee@linux.intel.com>
> >
> > Currently, kmalloc_index() can return -1, which can be
> > passed right to the kmalloc_caches[] array, cause a
>
> No kmalloc_index() cannot return -1 for the use case that you are
> considering here. The value passed as a size to
> kmalloc_slab is bounded by 2 * PAGE_SIZE and kmalloc_slab will only return
> -1 for sizes > 4M. So we will have to get machines with page sizes > 2M
> before this can be triggered.
Please don't make x86 centric assumptions like this. I was vaguely
thinking about hugepages in parisc. Like most risc machines, we have
(and have had for over a decade) a vast number of variable size pages
(actually from 4k to 64MB in power of 4 steps) and I think sparc is
similar, so I was wondering what to choose. You'd have been deeply
annoyed if I'd chosen 4MB and had slub fall over (again).
linux-arch cc'd just so everyone else is aware of these limitations when
they implement hugepages.
James
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kernel buffer overflow kmalloc_slab() fix
2011-05-20 12:02 ` [PATCH] kernel buffer overflow kmalloc_slab() fix James Bottomley
@ 2011-05-20 14:33 ` Christoph Lameter
2011-05-20 14:33 ` Christoph Lameter
2011-05-20 14:42 ` Christoph Lameter
0 siblings, 2 replies; 5+ messages in thread
From: Christoph Lameter @ 2011-05-20 14:33 UTC (permalink / raw)
To: James Bottomley
Cc: james_p_freyensee, linux-mm, gregkh, hari.k.kanigeri, linux-arch
On Fri, 20 May 2011, James Bottomley wrote:
> On Thu, 2011-05-19 at 15:51 -0500, Christoph Lameter wrote:
> > On Thu, 19 May 2011, james_p_freyensee@linux.intel.com wrote:
> >
> > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > >
> > > Currently, kmalloc_index() can return -1, which can be
> > > passed right to the kmalloc_caches[] array, cause a
> >
> > No kmalloc_index() cannot return -1 for the use case that you are
> > considering here. The value passed as a size to
> > kmalloc_slab is bounded by 2 * PAGE_SIZE and kmalloc_slab will only return
> > -1 for sizes > 4M. So we will have to get machines with page sizes > 2M
> > before this can be triggered.
>
> Please don't make x86 centric assumptions like this. I was vaguely
> thinking about hugepages in parisc. Like most risc machines, we have
> (and have had for over a decade) a vast number of variable size pages
> (actually from 4k to 64MB in power of 4 steps) and I think sparc is
> similar, so I was wondering what to choose. You'd have been deeply
> annoyed if I'd chosen 4MB and had slub fall over (again).
>
> linux-arch cc'd just so everyone else is aware of these limitations when
> they implement hugepages.
Well the simple solution would to put a BUG() in kmalloc_slab()
instead of returning -1 (whereupon the compiler will complain that we are
not returning a value). We tried the BUILD_BUG before but some compilers
are not playing ball there.
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kernel buffer overflow kmalloc_slab() fix
2011-05-20 14:33 ` Christoph Lameter
@ 2011-05-20 14:33 ` Christoph Lameter
2011-05-20 14:42 ` Christoph Lameter
1 sibling, 0 replies; 5+ messages in thread
From: Christoph Lameter @ 2011-05-20 14:33 UTC (permalink / raw)
To: James Bottomley
Cc: james_p_freyensee, linux-mm, gregkh, hari.k.kanigeri, linux-arch
On Fri, 20 May 2011, James Bottomley wrote:
> On Thu, 2011-05-19 at 15:51 -0500, Christoph Lameter wrote:
> > On Thu, 19 May 2011, james_p_freyensee@linux.intel.com wrote:
> >
> > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > >
> > > Currently, kmalloc_index() can return -1, which can be
> > > passed right to the kmalloc_caches[] array, cause a
> >
> > No kmalloc_index() cannot return -1 for the use case that you are
> > considering here. The value passed as a size to
> > kmalloc_slab is bounded by 2 * PAGE_SIZE and kmalloc_slab will only return
> > -1 for sizes > 4M. So we will have to get machines with page sizes > 2M
> > before this can be triggered.
>
> Please don't make x86 centric assumptions like this. I was vaguely
> thinking about hugepages in parisc. Like most risc machines, we have
> (and have had for over a decade) a vast number of variable size pages
> (actually from 4k to 64MB in power of 4 steps) and I think sparc is
> similar, so I was wondering what to choose. You'd have been deeply
> annoyed if I'd chosen 4MB and had slub fall over (again).
>
> linux-arch cc'd just so everyone else is aware of these limitations when
> they implement hugepages.
Well the simple solution would to put a BUG() in kmalloc_slab()
instead of returning -1 (whereupon the compiler will complain that we are
not returning a value). We tried the BUILD_BUG before but some compilers
are not playing ball there.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kernel buffer overflow kmalloc_slab() fix
2011-05-20 14:33 ` Christoph Lameter
2011-05-20 14:33 ` Christoph Lameter
@ 2011-05-20 14:42 ` Christoph Lameter
2011-05-20 18:02 ` J Freyensee
1 sibling, 1 reply; 5+ messages in thread
From: Christoph Lameter @ 2011-05-20 14:42 UTC (permalink / raw)
To: James Bottomley
Cc: james_p_freyensee, linux-mm, gregkh, hari.k.kanigeri, linux-arch,
Pekka Enberg
Subject: slub: Deal with hyperthetical case of PAGE_SIZE > 2M
kmalloc_index() currently returns -1 if the PAGE_SIZE is larger than 2M
which seems to cause some concern since the callers do not check for -1.
Insert a BUG() and add a comment to the -1 explaining that the code
cannot be reached.
Signed-off-by: Christoph Lameter <cl@linux.com>
---
include/linux/slub_def.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Index: linux-2.6/include/linux/slub_def.h
===================================================================
--- linux-2.6.orig/include/linux/slub_def.h 2011-05-20 09:37:02.000000000 -0500
+++ linux-2.6/include/linux/slub_def.h 2011-05-20 09:39:07.000000000 -0500
@@ -179,7 +179,8 @@ static __always_inline int kmalloc_index
if (size <= 4 * 1024) return 12;
/*
* The following is only needed to support architectures with a larger page
- * size than 4k.
+ * size than 4k. We need to support 2 * PAGE_SIZE here. So for a 64k page
+ * size we would have to go up to 128k.
*/
if (size <= 8 * 1024) return 13;
if (size <= 16 * 1024) return 14;
@@ -190,7 +191,8 @@ static __always_inline int kmalloc_index
if (size <= 512 * 1024) return 19;
if (size <= 1024 * 1024) return 20;
if (size <= 2 * 1024 * 1024) return 21;
- return -1;
+ BUG();
+ return -1; /* Will never be reached */
/*
* What we really wanted to do and cannot do because of compiler issues is:
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kernel buffer overflow kmalloc_slab() fix
2011-05-20 14:42 ` Christoph Lameter
@ 2011-05-20 18:02 ` J Freyensee
0 siblings, 0 replies; 5+ messages in thread
From: J Freyensee @ 2011-05-20 18:02 UTC (permalink / raw)
To: Christoph Lameter
Cc: James Bottomley, linux-mm, gregkh, hari.k.kanigeri, linux-arch,
Pekka Enberg
Thank you for the collaboration work on the fix. I like it.
Jay
On Fri, 2011-05-20 at 09:42 -0500, Christoph Lameter wrote:
> Subject: slub: Deal with hyperthetical case of PAGE_SIZE > 2M
>
> kmalloc_index() currently returns -1 if the PAGE_SIZE is larger than 2M
> which seems to cause some concern since the callers do not check for -1.
>
> Insert a BUG() and add a comment to the -1 explaining that the code
> cannot be reached.
>
> Signed-off-by: Christoph Lameter <cl@linux.com>
>
> ---
> include/linux/slub_def.h | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> Index: linux-2.6/include/linux/slub_def.h
> ===================================================================
> --- linux-2.6.orig/include/linux/slub_def.h 2011-05-20 09:37:02.000000000 -0500
> +++ linux-2.6/include/linux/slub_def.h 2011-05-20 09:39:07.000000000 -0500
> @@ -179,7 +179,8 @@ static __always_inline int kmalloc_index
> if (size <= 4 * 1024) return 12;
> /*
> * The following is only needed to support architectures with a larger page
> - * size than 4k.
> + * size than 4k. We need to support 2 * PAGE_SIZE here. So for a 64k page
> + * size we would have to go up to 128k.
> */
> if (size <= 8 * 1024) return 13;
> if (size <= 16 * 1024) return 14;
> @@ -190,7 +191,8 @@ static __always_inline int kmalloc_index
> if (size <= 512 * 1024) return 19;
> if (size <= 1024 * 1024) return 20;
> if (size <= 2 * 1024 * 1024) return 21;
> - return -1;
> + BUG();
> + return -1; /* Will never be reached */
>
> /*
> * What we really wanted to do and cannot do because of compiler issues is:
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-05-20 18:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1305834712-27805-2-git-send-email-james_p_freyensee@linux.intel.com>
[not found] ` <alpine.DEB.2.00.1105191550001.12530@router.home>
2011-05-20 12:02 ` [PATCH] kernel buffer overflow kmalloc_slab() fix James Bottomley
2011-05-20 14:33 ` Christoph Lameter
2011-05-20 14:33 ` Christoph Lameter
2011-05-20 14:42 ` Christoph Lameter
2011-05-20 18:02 ` J Freyensee
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).