malloc: Handle large malloc sizes in malloc_size()

[kib]

I think malloc_large() and this one need to check that round_page(size) does not overflow.

[wulf]

sys/kern/kern_malloc.c

1133

Should we return size intact if kernel config options DEBUG_REDZONE or DEBUG_MEMGUARD are set?

[markj]

In D45661#1041742, @kib wrote:

I think malloc_large() and this one need to check that round_page(size) does not overflow.

Hmm, kmem_malloc* also don't check for overflow. Presumably those functions and malloc_size() should KASSERT(round_page(size) >= size);?

sys/kern/kern_malloc.c

1133

For DEBUG_MEMGUARD, there is no way to do that correctly, since we don't know whether memguard is enabled for the allocation. But, the current behaviour is safe in that the returned value is always less than or equal to the true allocation size.

For DEBUG_REDZONE, what is the right behaviour? Should we just return size (so that consumers do not trigger false positives by attempting to write outside of the allocation bounds) or should we try to implement it properly? I would presume the former, but I don't know how this function will be used.

BTW, I think DEBUG_REDZONE and DEBUG_MEMGUARD are of limited use now that we have kernel sanitizers.

[kib]

In D45661#1042253, @markj wrote:

In D45661#1041742, @kib wrote:

I think malloc_large() and this one need to check that round_page(size) does not overflow.

Hmm, kmem_malloc* also don't check for overflow. Presumably those functions and malloc_size() should KASSERT(round_page(size) >= size);?

It would be useful, but only for debug kernels. For instance, an unhandled unbound allocation leaking from some device' ioctl is my worry there.

[imp]

In D45661#1042261, @kib wrote:

In D45661#1042253, @markj wrote:

In D45661#1041742, @kib wrote:

I think malloc_large() and this one need to check that round_page(size) does not overflow.

Hmm, kmem_malloc* also don't check for overflow. Presumably those functions and malloc_size() should KASSERT(round_page(size) >= size);?

It would be useful, but only for debug kernels. For instance, an unhandled unbound allocation leaking from some device' ioctl is my worry there.

I think we need the stronger if. I don't think we bounds check enough down the stack for this to be a simple assert

[markj]

In D45661#1042261, @kib wrote:

In D45661#1042253, @markj wrote:

In D45661#1041742, @kib wrote:

I think malloc_large() and this one need to check that round_page(size) does not overflow.

Hmm, kmem_malloc* also don't check for overflow. Presumably those functions and malloc_size() should KASSERT(round_page(size) >= size);?

It would be useful, but only for debug kernels. For instance, an unhandled unbound allocation leaking from some device' ioctl is my worry there.

Indeed, we have had examples like this, e.g., https://syzkaller.appspot.com/bug?extid=72022fa9163fa958b66c

In a debug kernel, vmem(size == 0) will trigger an assertion failure, so the bug is caught easily, but I'm not sure what happens otherwise. I will write a patch.

[wulf]

Any plans to commit?

[markj]

In D45661#1042261, @kib wrote:

In D45661#1042253, @markj wrote:

In D45661#1041742, @kib wrote:

I think malloc_large() and this one need to check that round_page(size) does not overflow.

Hmm, kmem_malloc* also don't check for overflow. Presumably those functions and malloc_size() should KASSERT(round_page(size) >= size);?

It would be useful, but only for debug kernels. For instance, an unhandled unbound allocation leaking from some device' ioctl is my worry there.

https://reviews.freebsd.org/D46110 and https://reviews.freebsd.org/D46111