Before, if the total alloc size is overflow,
we just return NULL like alloc fail. But they
are two different type problems. The former looks
more like a programming problem. So add a warning
here.
Signed-off-by: Yang Bai <[email protected]>
---
include/linux/slab.h | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 573c809..5865237 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -242,8 +242,10 @@ size_t ksize(const void *);
*/
static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
{
- if (size != 0 && n > ULONG_MAX / size)
+ if (size != 0 && n > ULONG_MAX / size) {
+ WARN(1, "Alloc memory size (%lu * %lu) overflow.", n, size);
return NULL;
+ }
return __kmalloc(n * size, flags | __GFP_ZERO);
}
--
1.7.9
On Tue, 14 Feb 2012, Yang Bai wrote:
> Before, if the total alloc size is overflow,
> we just return NULL like alloc fail. But they
> are two different type problems. The former looks
> more like a programming problem. So add a warning
> here.
>
> Signed-off-by: Yang Bai <[email protected]>
> ---
> include/linux/slab.h | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 573c809..5865237 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -242,8 +242,10 @@ size_t ksize(const void *);
> */
> static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> {
> - if (size != 0 && n > ULONG_MAX / size)
> + if (size != 0 && n > ULONG_MAX / size) {
> + WARN(1, "Alloc memory size (%lu * %lu) overflow.", n, size);
> return NULL;
> + }
> return __kmalloc(n * size, flags | __GFP_ZERO);
> }
Did you check how much kernel text size increases? I'm pretty sure we'd
need to wrap this with CONFIG_SLAB_OVERFLOW ifdef.
Pekka
On Tue, Feb 14, 2012 at 3:31 PM, Pekka Enberg <[email protected]> wrote:
> On Tue, 14 Feb 2012, Yang Bai wrote:
>
> Did you check how much kernel text size increases? I'm pretty sure we'd need
> to wrap this with CONFIG_SLAB_OVERFLOW ifdef.
>
> Pekka
Hi Pekka,
I did not find anything like SLAB_OVERFLOW using grep. Could you
explain it more in detail?
--
"""
Keep It Simple,Stupid.
"""
Chinese Name: 白杨
Nick Name: Hamo
Homepage: http://hamobai.com/
GPG KEY ID: 0xA4691A33
Key fingerprint = 09D5 2D78 8E2B 0995 CF8E 4331 33C4 3D24 A469 1A33
On Tue, 14 Feb 2012, Yang Bai wrote:
> I did not find anything like SLAB_OVERFLOW using grep. Could you
> explain it more in detail?
You should add a new config option to lib/Kconfig.debug and wrap the debug
check with it.
Pekka
On Tue, 14 Feb 2012 15:28:19 +0800 Yang Bai <[email protected]> wrote:
> Before, if the total alloc size is overflow,
> we just return NULL like alloc fail. But they
> are two different type problems. The former looks
> more like a programming problem. So add a warning
> here.
>
> Signed-off-by: Yang Bai <[email protected]>
> ---
> include/linux/slab.h | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 573c809..5865237 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -242,8 +242,10 @@ size_t ksize(const void *);
> */
> static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> {
> - if (size != 0 && n > ULONG_MAX / size)
> + if (size != 0 && n > ULONG_MAX / size) {
> + WARN(1, "Alloc memory size (%lu * %lu) overflow.", n, size);
> return NULL;
> + }
> return __kmalloc(n * size, flags | __GFP_ZERO);
> }
One of the applications of kcalloc() is to prevent userspace from
causing a multiplicative overflow (and then perhaps causing an
overwrite beyond the end of the allocated memory).
With this patch, we've just handed the user a way of spamming the logs
at 1MHz. This is bad.
Also, please let's not randomly add debug stuff in places where we've
never demonstrated a need for it.
On Tue, Feb 14, 2012 at 4:53 PM, Andrew Morton
<[email protected]> wrote:
> On Tue, 14 Feb 2012 15:28:19 +0800 Yang Bai <[email protected]> wrote:
>
>
> One of the applications of kcalloc() is to prevent userspace from
> causing a multiplicative overflow (and then perhaps causing an
> overwrite beyond the end of the allocated memory).
>
> With this patch, we've just handed the user a way of spamming the logs
> at 1MHz. This is bad.
>
>
> Also, please let's not randomly add debug stuff in places where we've
> never demonstrated a need for it.
Ok. Please just drop this patch.
Thanks.
--
"""
Keep It Simple,Stupid.
"""
Chinese Name: 白杨
Nick Name: Hamo
Homepage: http://hamobai.com/
GPG KEY ID: 0xA4691A33
Key fingerprint = 09D5 2D78 8E2B 0995 CF8E 4331 33C4 3D24 A469 1A33
On Tue, 14 Feb 2012, Yang Bai wrote:
> Before, if the total alloc size is overflow,
> we just return NULL like alloc fail. But they
> are two different type problems. The former looks
> more like a programming problem. So add a warning
> here.
Acked-by: Christoph Lameter <[email protected]>
Would be better to remove kcalloc and provide a generalized array size
calculation function that does the WARN(). That would also work for all
other variants zeroed or NUMA node spec etc etc.
On Tue, 14 Feb 2012, Pekka Enberg wrote:
> Did you check how much kernel text size increases? I'm pretty sure we'd need
> to wrap this with CONFIG_SLAB_OVERFLOW ifdef.
Remove the inlining? This function is rarely called and not performance
critical.
On Tue, 14 Feb 2012, Andrew Morton wrote:
> One of the applications of kcalloc() is to prevent userspace from
> causing a multiplicative overflow (and then perhaps causing an
> overwrite beyond the end of the allocated memory).
>
> With this patch, we've just handed the user a way of spamming the logs
> at 1MHz. This is bad.
Well there is WARN_ON_ONCE too to prevent that.