Mailing List Archive: Linux: Kernel

integer overflows in kernel/relay.c

Index | Next | Previous | View Threaded

dan.carpenter at oracle

Feb 7, 2012, 6:11 AM

Post #1 of 6 (54 views)
Permalink

integer overflows in kernel/relay.c

My static checker is warning about integer overflows in kernel/relay.c

relay_create_buf()
170
171 buf->padding = kmalloc(chan->n_subbufs * sizeof(size_t *), GFP_KERNEL);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This can only overflow on 32bit systems.

172 if (!buf->padding)
173 goto free_buf;
174

relay_open()
582 chan->version = RELAYFS_CHANNEL_VERSION;
583 chan->n_subbufs = n_subbufs;
584 chan->subbuf_size = subbuf_size;
585 chan->alloc_size = FIX_SIZE(subbuf_size * n_subbufs);
^^^^^^^^^^^^^^^^^^^^^^^
586 chan->parent = parent;

These come from the user in blk_trace_setup() and they aren't capped.
I'm not sure what the maximum size to use is.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

axboe at kernel

Feb 8, 2012, 12:34 AM

Post #2 of 6 (48 views)
Permalink

Re: integer overflows in kernel/relay.c [In reply to]

On 02/07/2012 03:11 PM, Dan Carpenter wrote:
> My static checker is warning about integer overflows in kernel/relay.c
>
> relay_create_buf()
> 170
> 171 buf->padding = kmalloc(chan->n_subbufs * sizeof(size_t *), GFP_KERNEL);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This can only overflow on 32bit systems.

Correct

> 172 if (!buf->padding)
> 173 goto free_buf;
> 174
>
> relay_open()
> 582 chan->version = RELAYFS_CHANNEL_VERSION;
> 583 chan->n_subbufs = n_subbufs;
> 584 chan->subbuf_size = subbuf_size;
> 585 chan->alloc_size = FIX_SIZE(subbuf_size * n_subbufs);
> ^^^^^^^^^^^^^^^^^^^^^^^
> 586 chan->parent = parent;
>
> These come from the user in blk_trace_setup() and they aren't capped.
> I'm not sure what the maximum size to use is.

They are both u32 types, so can overflow on 32-bit as well. By default,
blktrace is using 4 for n_subbufs and 512k for subbuf_size, but they are
configurable. As a fix, I would suggest just checking if the products
overflow, and if they do, return an error. That's better than imposing
some hard limits. In reality, only a malicious users would trigger
these.

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

akpm at linux-foundation

Feb 8, 2012, 2:25 PM

Post #3 of 6 (44 views)
Permalink

Re: integer overflows in kernel/relay.c [In reply to]

On Wed, 08 Feb 2012 09:34:16 +0100
Jens Axboe <axboe [at] kernel> wrote:

> On 02/07/2012 03:11 PM, Dan Carpenter wrote:
> > My static checker is warning about integer overflows in kernel/relay.c
> >
> > relay_create_buf()
> > 170
> > 171 buf->padding = kmalloc(chan->n_subbufs * sizeof(size_t *), GFP_KERNEL);
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > This can only overflow on 32bit systems.
>
> Correct

This is a relatively common problem. Converting a kzalloc() call to
kcalloc() fixes it. But we do not have a non-zeroing version of
kcalloc().

Please, someone complete this patch and send it to me!

--- a/include/linux/slab.h~a
+++ a/include/linux/slab.h
@@ -240,11 +240,16 @@ size_t ksize(const void *);
* for general use, and so are not documented here. For a full list of
* potential flags, always refer to linux/gfp.h.
*/
-static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
+static inline void *wtf_do_i_call_this(size_t n, size_t size, gfp_t flags)
{
if (size != 0 && n > ULONG_MAX / size)
return NULL;
- return __kmalloc(n * size, flags | __GFP_ZERO);
+ return __kmalloc(n * size, flags);
+}
+
+static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
+{
+ return wtf_do_i_call_this(n, size, flags | __GFP_ZERO);
}

#if !defined(CONFIG_NUMA) && !defined(CONFIG_SLOB)
_

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

axboe at kernel

Feb 9, 2012, 4:41 AM

Post #4 of 6 (43 views)
Permalink

Re: integer overflows in kernel/relay.c [In reply to]

On 02/08/2012 11:25 PM, Andrew Morton wrote:
> On Wed, 08 Feb 2012 09:34:16 +0100
> Jens Axboe <axboe [at] kernel> wrote:
>
>> On 02/07/2012 03:11 PM, Dan Carpenter wrote:
>>> My static checker is warning about integer overflows in kernel/relay.c
>>>
>>> relay_create_buf()
>>> 170
>>> 171 buf->padding = kmalloc(chan->n_subbufs * sizeof(size_t *), GFP_KERNEL);
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> This can only overflow on 32bit systems.
>>
>> Correct
>
> This is a relatively common problem. Converting a kzalloc() call to
> kcalloc() fixes it. But we do not have a non-zeroing version of
> kcalloc().
>
> Please, someone complete this patch and send it to me!
>
>
> --- a/include/linux/slab.h~a
> +++ a/include/linux/slab.h
> @@ -240,11 +240,16 @@ size_t ksize(const void *);
> * for general use, and so are not documented here. For a full list of
> * potential flags, always refer to linux/gfp.h.
> */
> -static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> +static inline void *wtf_do_i_call_this(size_t n, size_t size, gfp_t flags)

kzcalloc()? Either that, or long_and_verbose().

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

penberg at kernel

Feb 9, 2012, 4:56 AM

Post #5 of 6 (43 views)
Permalink

Re: integer overflows in kernel/relay.c [In reply to]

On Thu, Feb 9, 2012 at 12:25 AM, Andrew Morton
<akpm [at] linux-foundation> wrote:
> On Wed, 08 Feb 2012 09:34:16 +0100
> Jens Axboe <axboe [at] kernel> wrote:
>
>> On 02/07/2012 03:11 PM, Dan Carpenter wrote:
>> > My static checker is warning about integer overflows in kernel/relay.c
>> >
>> > relay_create_buf()
>> > � �170
>> > � �171 � � � � �buf->padding = kmalloc(chan->n_subbufs * sizeof(size_t *), GFP_KERNEL);
>> > � � � � � � � � � � � � � � � � � � � �^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> > This can only overflow on 32bit systems.
>>
>> Correct
>
> This is a relatively common problem. �Converting a kzalloc() call to
> kcalloc() fixes it. �But we do not have a non-zeroing version of
> kcalloc().
>
> Please, someone complete this patch and send it to me!
>
>
> --- a/include/linux/slab.h~a
> +++ a/include/linux/slab.h
> @@ -240,11 +240,16 @@ size_t ksize(const void *);
> �* for general use, and so are not documented here. For a full list of
> �* potential flags, always refer to linux/gfp.h.
> �*/
> -static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> +static inline void *wtf_do_i_call_this(size_t n, size_t size, gfp_t flags)

I'd call it kmalloc_array().

> �{
> � � � �if (size != 0 && n > ULONG_MAX / size)
> � � � � � � � �return NULL;
> - � � � return __kmalloc(n * size, flags | __GFP_ZERO);
> + � � � return __kmalloc(n * size, flags);
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

akpm at linux-foundation

Feb 9, 2012, 9:39 AM

Post #6 of 6 (42 views)
Permalink

Re: integer overflows in kernel/relay.c [In reply to]

On Thu, 09 Feb 2012 13:41:38 +0100 Jens Axboe <axboe [at] kernel> wrote:

> On 02/08/2012 11:25 PM, Andrew Morton wrote:
> > On Wed, 08 Feb 2012 09:34:16 +0100
> > Jens Axboe <axboe [at] kernel> wrote:
> >
> >> On 02/07/2012 03:11 PM, Dan Carpenter wrote:
> >>> My static checker is warning about integer overflows in kernel/relay.c
> >>>
> >>> relay_create_buf()
> >>> 170
> >>> 171 buf->padding = kmalloc(chan->n_subbufs * sizeof(size_t *), GFP_KERNEL);
> >>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >>> This can only overflow on 32bit systems.
> >>
> >> Correct
> >
> > This is a relatively common problem. Converting a kzalloc() call to
> > kcalloc() fixes it. But we do not have a non-zeroing version of
> > kcalloc().
> >
> > Please, someone complete this patch and send it to me!
> >
> >
> > --- a/include/linux/slab.h~a
> > +++ a/include/linux/slab.h
> > @@ -240,11 +240,16 @@ size_t ksize(const void *);
> > * for general use, and so are not documented here. For a full list of
> > * potential flags, always refer to linux/gfp.h.
> > */
> > -static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> > +static inline void *wtf_do_i_call_this(size_t n, size_t size, gfp_t flags)
>
> kzcalloc()? Either that, or long_and_verbose().

I would need to be k_not_z_calloc() because it's the non-zeroing version.

kcallocn()? knalloc()? kncalloc()?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads