Introduce kcalloc() in linux/slab.h that is used to replace snd_calloc() and
snd_magic_calloc().
Signed-off-by: Pekka Enberg <[email protected]>
include/linux/slab.h | 1 +
mm/slab.c | 15 +++++++++++++++
2 files changed, 16 insertions(+)
--- linux-2.6.6/include/linux/slab.h 2004-06-08 23:54:41.382519192 +0300
+++ alsa-2.6.6/include/linux/slab.h 2004-06-08 21:36:12.951592680 +0300
@@ -95,6 +95,7 @@ found:
return __kmalloc(size, flags);
}
+extern void *kcalloc(size_t, int);
extern void kfree(const void *);
extern unsigned int ksize(const void *);
--- linux-2.6.6/mm/slab.c 2004-06-08 23:57:30.713776928 +0300
+++ alsa-2.6.6/mm/slab.c 2004-06-08 21:42:18.000000000 +0300
@@ -2332,6 +2332,21 @@ void kmem_cache_free (kmem_cache_t *cach
EXPORT_SYMBOL(kmem_cache_free);
/**
+ * kcalloc - allocate memory. The memory is set to zero.
+ * @size: how many bytes of memory are required.
+ * @flags: the type of memory to allocate.
+ */
+void *kcalloc(size_t size, int flags)
+{
+ void *ret = kmalloc(size, flags);
+ if (ret)
+ memset(ret, 0, size);
+ return ret;
+}
+
+EXPORT_SYMBOL(kcalloc);
+
+/**
* kfree - free previously allocated memory
* @objp: pointer returned by kmalloc.
*
* Pekka J Enberg ([email protected]) wrote:
> /**
> + * kcalloc - allocate memory. The memory is set to zero.
> + * @size: how many bytes of memory are required.
> + * @flags: the type of memory to allocate.
> + */
> +void *kcalloc(size_t size, int flags)
> +{
> + void *ret = kmalloc(size, flags);
> + if (ret)
> + memset(ret, 0, size);
> + return ret;
> +}
This looks more like the kzalloc() that pops up every now and then.
Wouldn't it make more sense to give kcalloc() a true calloc() style
interface?
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
On Wed, 2004-06-09 at 21:34, Chris Wright wrote:
> This looks more like the kzalloc() that pops up every now and then.
> Wouldn't it make more sense to give kcalloc() a true calloc() style
> interface?
I can go either way. Takashi, do you want me to update the ALSA patches
to use this version?
Pekka
diff -urN linux-2.6.6/include/linux/slab.h kcalloc-2.6.6/include/linux/slab.h
--- linux-2.6.6/include/linux/slab.h 2004-06-09 22:56:11.874249056 +0300
+++ kcalloc-2.6.6/include/linux/slab.h 2004-06-09 23:03:10.597593432 +0300
@@ -95,6 +95,7 @@
return __kmalloc(size, flags);
}
+extern void *kcalloc(size_t, size_t, int);
extern void kfree(const void *);
extern unsigned int ksize(const void *);
diff -urN linux-2.6.6/mm/slab.c kcalloc-2.6.6/mm/slab.c
--- linux-2.6.6/mm/slab.c 2004-06-09 22:59:13.081701336 +0300
+++ kcalloc-2.6.6/mm/slab.c 2004-06-09 23:07:51.262925816 +0300
@@ -2332,6 +2332,22 @@
EXPORT_SYMBOL(kmem_cache_free);
/**
+ * kcalloc - allocate memory for an array. The memory is set to zero.
+ * @n: number of elements.
+ * @size: element size.
+ * @flags: the type of memory to allocate.
+ */
+void *kcalloc(size_t n, size_t size, int flags)
+{
+ void *ret = kmalloc(n * size, flags);
+ if (ret)
+ memset(ret, 0, n * size);
+ return ret;
+}
+
+EXPORT_SYMBOL(kcalloc);
+
+/**
* kfree - free previously allocated memory
* @objp: pointer returned by kmalloc.
*
> + */
> +void *kcalloc(size_t n, size_t size, int flags)
> +{
> + void *ret = kmalloc(n * size, flags);
how about making sure n*size doesn't overflow an int in this function?
We had a few security holes due to that happening a while ago; might as
well prevent it from happening entirely
On Wed, 09 Jun 2004 22:21:26 +0200, Arjan van de Ven said:
> > + */
> > +void *kcalloc(size_t n, size_t size, int flags)
> > +{
> > + void *ret = kmalloc(n * size, flags);
>
> how about making sure n*size doesn't overflow an int in this function?
> We had a few security holes due to that happening a while ago; might as
> well prevent it from happening entirely
Do we want 'int', or is some other value (size_t? u32?) a better bet? (I see on
some of the 64-bit boxes a compat_size_t for 32-bits as well, which hints at
the problems here....
I'm worried that 'int' will Do The Wrong Thing when it runs into stuff like
this from asm-i386/types.h:
/* DMA addresses come in generic and 64-bit flavours. */
#ifdef CONFIG_HIGHMEM64G
typedef u64 dma_addr_t;
#else
typedef u32 dma_addr_t;
#endif
typedef u64 dma64_addr_t;
Are there any platforms where 'int' and 'max reasonable kmalloc size' have the
same number of bits?
On Wed, 2004-06-09 at 23:21, Arjan van de Ven wrote:
> how about making sure n*size doesn't overflow an int in this function?
> We had a few security holes due to that happening a while ago; might as
> well prevent it from happening entirely
Sure.
Pekka
diff -urN linux-2.6.6/include/linux/slab.h kcalloc-2.6.6/include/linux/slab.h
--- linux-2.6.6/include/linux/slab.h 2004-06-09 22:56:11.874249056 +0300
+++ kcalloc-2.6.6/include/linux/slab.h 2004-06-09 23:03:10.597593432 +0300
@@ -95,6 +95,7 @@
return __kmalloc(size, flags);
}
+extern void *kcalloc(size_t, size_t, int);
extern void kfree(const void *);
extern unsigned int ksize(const void *);
diff -urN linux-2.6.6/mm/slab.c kcalloc-2.6.6/mm/slab.c
--- linux-2.6.6/mm/slab.c 2004-06-09 22:59:13.081701336 +0300
+++ kcalloc-2.6.6/mm/slab.c 2004-06-09 23:50:06.592497136 +0300
@@ -2332,6 +2332,25 @@
EXPORT_SYMBOL(kmem_cache_free);
/**
+ * kcalloc - allocate memory for an array. The memory is set to zero.
+ * @n: number of elements.
+ * @size: element size.
+ * @flags: the type of memory to allocate.
+ */
+void *kcalloc(size_t n, size_t size, int flags)
+{
+ if (n != 0 && size > INT_MAX / n)
+ return NULL;
+
+ void *ret = kmalloc(n * size, flags);
+ if (ret)
+ memset(ret, 0, n * size);
+ return ret;
+}
+
+EXPORT_SYMBOL(kcalloc);
+
+/**
* kfree - free previously allocated memory
* @objp: pointer returned by kmalloc.
*
* Arjan van de Ven ([email protected]) wrote:
>
> > + */
> > +void *kcalloc(size_t n, size_t size, int flags)
> > +{
> > + void *ret = kmalloc(n * size, flags);
>
> how about making sure n*size doesn't overflow an int in this function?
> We had a few security holes due to that happening a while ago; might as
> well prevent it from happening entirely
Nice point. Too bad all we can return is NULL, it'd be nice to know the
overflow was the reason. Do we plop a WARN_ON() in there for a while?
Something like below (w/out any WARN_ON right now)?
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
diff -urN linux-2.6.6/include/linux/slab.h kcalloc-2.6.6/include/linux/slab.h
--- linux-2.6.6/include/linux/slab.h 2004-06-09 22:56:11.874249056 +0300
+++ kcalloc-2.6.6/include/linux/slab.h 2004-06-09 23:03:10.597593432 +0300
@@ -95,6 +95,7 @@
return __kmalloc(size, flags);
}
+extern void *kcalloc(size_t, size_t, int);
extern void kfree(const void *);
extern unsigned int ksize(const void *);
diff -urN linux-2.6.6/mm/slab.c kcalloc-2.6.6/mm/slab.c
--- linux-2.6.6/mm/slab.c 2004-06-09 22:59:13.081701336 +0300
+++ kcalloc-2.6.6/mm/slab.c 2004-06-09 23:07:51.262925816 +0300
@@ -2332,6 +2332,28 @@
EXPORT_SYMBOL(kmem_cache_free);
/**
+ * kcalloc - allocate memory for an array. The memory is set to zero.
+ * @n: number of elements.
+ * @size: element size.
+ * @flags: the type of memory to allocate.
+ */
+void *kcalloc(size_t n, size_t size, int flags)
+{
+ void *ret = NULL;
+
+ /* check for overflow */
+ if (n > UINT_MAX/size)
+ return ret;
+
+ ret = kmalloc(n * size, flags);
+ if (ret)
+ memset(ret, 0, n * size);
+ return ret;
+}
+
+EXPORT_SYMBOL(kcalloc);
+
+/**
* kfree - free previously allocated memory
* @objp: pointer returned by kmalloc.
*
* Pekka Enberg ([email protected]) wrote:
> +void *kcalloc(size_t n, size_t size, int flags)
> +{
> + if (n != 0 && size > INT_MAX / n)
> + return NULL;
Yup, I forgot to add divide by zero check.
> + void *ret = kmalloc(n * size, flags);
This is only C99, and will make some compilers spit up warning.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
On Wed, Jun 09, 2004 at 11:57:43PM +0300, Pekka Enberg wrote:
> On Wed, 2004-06-09 at 23:21, Arjan van de Ven wrote:
> > how about making sure n*size doesn't overflow an int in this function?
> > We had a few security holes due to that happening a while ago; might as
> > well prevent it from happening entirely
>
> Sure.
>
> Pekka
>
> diff -urN linux-2.6.6/include/linux/slab.h kcalloc-2.6.6/include/linux/slab.h
> --- linux-2.6.6/include/linux/slab.h 2004-06-09 22:56:11.874249056 +0300
> +++ kcalloc-2.6.6/include/linux/slab.h 2004-06-09 23:03:10.597593432 +0300
> @@ -95,6 +95,7 @@
> return __kmalloc(size, flags);
> }
>
> +extern void *kcalloc(size_t, size_t, int);
> extern void kfree(const void *);
> extern unsigned int ksize(const void *);
>
> diff -urN linux-2.6.6/mm/slab.c kcalloc-2.6.6/mm/slab.c
> --- linux-2.6.6/mm/slab.c 2004-06-09 22:59:13.081701336 +0300
> +++ kcalloc-2.6.6/mm/slab.c 2004-06-09 23:50:06.592497136 +0300
> @@ -2332,6 +2332,25 @@
> EXPORT_SYMBOL(kmem_cache_free);
>
> /**
> + * kcalloc - allocate memory for an array. The memory is set to zero.
> + * @n: number of elements.
> + * @size: element size.
> + * @flags: the type of memory to allocate.
> + */
> +void *kcalloc(size_t n, size_t size, int flags)
> +{
> + if (n != 0 && size > INT_MAX / n)
> + return NULL;
> +
> + void *ret = kmalloc(n * size, flags);
> + if (ret)
> + memset(ret, 0, n * size);
> + return ret;
> +}
ok I like it ;)
only question is what n==0 means, might as well short-circuit that but it's
optional
On Wed, Jun 09, 2004 at 11:57:43PM +0300, Pekka Enberg wrote:
> > +void *kcalloc(size_t n, size_t size, int flags)
> > +{
> > + if (n != 0 && size > INT_MAX / n)
> > + return NULL;
> > +
> > + void *ret = kmalloc(n * size, flags);
> > + if (ret)
> > + memset(ret, 0, n * size);
> > + return ret;
> > +}
On Wed, 2004-06-09 at 23:59, Arjan van de Ven wrote:
> ok I like it ;)
>
> only question is what n==0 means, might as well short-circuit that but it's
> optional
Nah, I don't see the point. Now if I can only convince the ALSA guys to
switch to this... =)
Pekka
On Thu, 2004-06-10 at 00:00, Chris Wright wrote:
> > + void *ret = kmalloc(n * size, flags);
>
> This is only C99, and will make some compilers spit up warning.
Indeed, I'd better fix that as well then ;)
Pekka
diff -urN linux-2.6.6/include/linux/slab.h kcalloc-2.6.6/include/linux/slab.h
--- linux-2.6.6/include/linux/slab.h 2004-06-09 22:56:11.874249056 +0300
+++ kcalloc-2.6.6/include/linux/slab.h 2004-06-09 23:03:10.597593432 +0300
@@ -95,6 +95,7 @@
return __kmalloc(size, flags);
}
+extern void *kcalloc(size_t, size_t, int);
extern void kfree(const void *);
extern unsigned int ksize(const void *);
diff -urN linux-2.6.6/mm/slab.c kcalloc-2.6.6/mm/slab.c
--- linux-2.6.6/mm/slab.c 2004-06-09 22:59:13.081701336 +0300
+++ kcalloc-2.6.6/mm/slab.c 2004-06-10 00:08:44.004624672 +0300
@@ -2332,6 +2332,27 @@
EXPORT_SYMBOL(kmem_cache_free);
/**
+ * kcalloc - allocate memory for an array. The memory is set to zero.
+ * @n: number of elements.
+ * @size: element size.
+ * @flags: the type of memory to allocate.
+ */
+void *kcalloc(size_t n, size_t size, int flags)
+{
+ void *ret = NULL;
+
+ if (n != 0 && size > INT_MAX / n)
+ return ret;
+
+ ret = kmalloc(n * size, flags);
+ if (ret)
+ memset(ret, 0, n * size);
+ return ret;
+}
+
+EXPORT_SYMBOL(kcalloc);
+
+/**
* kfree - free previously allocated memory
* @objp: pointer returned by kmalloc.
*
On Wed, 9 Jun 2004, Pekka Enberg wrote:
> +void *kcalloc(size_t n, size_t size, int flags)
> +{
> + if (n != 0 && size > INT_MAX / n)
> + return NULL;
division isn't very efficient :) it's typically a 40+ cycle operation.
there's almost certainly more efficient ways to do this with per-target
assembly... but you should probably just crib the code from glibc which
avoids division for small allocations:
/* size_t is unsigned so the behavior on overflow is defined. */
bytes = n * elem_size;
#define HALF_INTERNAL_SIZE_T \
(((INTERNAL_SIZE_T) 1) << (8 * sizeof (INTERNAL_SIZE_T) / 2))
if (__builtin_expect ((n | elem_size) >= HALF_INTERNAL_SIZE_T, 0)) {
if (elem_size != 0 && bytes / elem_size != n) {
MALLOC_FAILURE_ACTION;
return 0;
}
}
-dean
On Wed, Jun 09, 2004 at 10:08:34PM -0700, dean gaudet wrote:
> > +void *kcalloc(size_t n, size_t size, int flags)
> > +{
> > + if (n != 0 && size > INT_MAX / n)
> > + return NULL;
>
> division isn't very efficient :) it's typically a 40+ cycle operation.
>
> there's almost certainly more efficient ways to do this with per-target
> assembly... but you should probably just crib the code from glibc which
> avoids division for small allocations:
>
> /* size_t is unsigned so the behavior on overflow is defined. */
> bytes = n * elem_size;
> #define HALF_INTERNAL_SIZE_T \
> (((INTERNAL_SIZE_T) 1) << (8 * sizeof (INTERNAL_SIZE_T) / 2))
> if (__builtin_expect ((n | elem_size) >= HALF_INTERNAL_SIZE_T, 0)) {
> if (elem_size != 0 && bytes / elem_size != n) {
> MALLOC_FAILURE_ACTION;
> return 0;
> }
> }
Considering this isn't exactly a cycle-critical piece of code,
I'd choose readability over saving 40 cycles which is probably
lost in the noise of every other operation we do in that allocation.
Dave
At Thu, 10 Jun 2004 00:07:49 +0300,
Pekka Enberg wrote:
>
> On Wed, Jun 09, 2004 at 11:57:43PM +0300, Pekka Enberg wrote:
> > > +void *kcalloc(size_t n, size_t size, int flags)
> > > +{
> > > + if (n != 0 && size > INT_MAX / n)
> > > + return NULL;
> > > +
> > > + void *ret = kmalloc(n * size, flags);
> > > + if (ret)
> > > + memset(ret, 0, n * size);
> > > + return ret;
> > > +}
>
> On Wed, 2004-06-09 at 23:59, Arjan van de Ven wrote:
> > ok I like it ;)
> >
> > only question is what n==0 means, might as well short-circuit that but it's
> > optional
>
> Nah, I don't see the point. Now if I can only convince the ALSA guys to
> switch to this... =)
I don't think the shortcut is necessary, too.
By calling kmalloc with 0, the same behavior as kmalloc is
guaranteed.
BTW, I agree with conversion to the standard calloc() style :)
thanks,
Takashi
Hi,
On 6/11/2004, "Takashi Iwai" <[email protected]> wrote:
> I don't think the shortcut is necessary, too.
> By calling kmalloc with 0, the same behavior as kmalloc is
> guaranteed.
Agreed. I'm wondering if the overflow case should set size to zero and call
kmalloc() as well.
Pekka