From: Eric Dumazet <edumazet@google.com>

Sent: 04 September 2023 10:06
To: David Laight <redacted>
Cc: David S . Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
[off-list ref]; netdev@vger.kernel.org; eric.dumazet@gmail.com; syzbot
[off-list ref]; Kyle Zeng [off-list ref]; Kees Cook [off-list ref];
Vlastimil Babka [off-list ref]
Subject: Re: [PATCH net] net: deal with integer overflows in kmalloc_reserve()

On Mon, Sep 4, 2023 at 10:41 AM David Laight [off-list ref] wrote:

quoted

From: Eric Dumazet

quoted

Sent: 31 August 2023 19:38

Blamed commit changed:
    ptr = kmalloc(size);
    if (ptr)
      size = ksize(ptr);

to:
    size = kmalloc_size_roundup(size);
    ptr = kmalloc(size);

This allowed various crash as reported by syzbot [1]
and Kyle Zeng.

Problem is that if @size is bigger than 0x80000001,
kmalloc_size_roundup(size) returns 2^32.

kmalloc_reserve() uses a 32bit variable (obj_size),
so 2^32 is truncated to 0.

Can this happen on 32bit arch?
In that case kmalloc_size_roundup() will return 0.

Maybe, but this would be a bug in kmalloc_size_roundup()

That contains:
	/* Short-circuit saturated "too-large" case. */
	if (unlikely(size == SIZE_MAX))
		return SIZE_MAX;

It can also return 0 on failure, I can't remember if kmalloc(0)
is guaranteed to be NULL (malloc(0) can do 'other things').

Which is entirely hopeless since MAX_SIZE is (size_t)-1.

IIRC kmalloc() has a size limit (max 'order' of pages) so
kmalloc_size_roundup() ought check for that (or its max value).

The final:
	/* The flags don't matter since size_index is common to all. */
	c = kmalloc_slab(size, GFP_KERNEL);
	return c ? c->object_size : 0;
probably ought to return size if c is even NULL.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)