Re: [PATCH net-next v3 1/4] net: devmem: allow rx-buf-size > PAGE_SIZE per dmabuf binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] Integer overflow and signedness bug leading to a negative array index and out-of-bounds memory corruption on 32-bit architectures.

Pre-existing issues:
- [High] Sleeping in atomic context due to kvfree() inside an RCU read-side critical section.
--

commit b8e27f9eee44549dc6869beb99adddc8150160f6
Author: Bobby Eshleman <bobbyeshleman@meta.com>

net: devmem: allow rx-buf-size > PAGE_SIZE per dmabuf binding

Every devmem dmabuf binding today hands the page_pool PAGE_SIZE niovs.
This caps a single RX descriptor at PAGE_SIZE, burning CPU on buffer
churn for large flows.

Add a bind-time netlink attribute, NETDEV_A_DMABUF_RX_BUF_SIZE, that
lets userspace request a larger niov size. The value must be a power of
two >= PAGE_SIZE.

[ ... ]

> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index 957d6b96216b3..3ce3cc14bec07 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c

[ ... ]

> @@ -93,13 +93,14 @@ net_devmem_alloc_dmabuf(struct net_devmem_dmabuf_binding *binding)
>  	ssize_t offset;
>  	ssize_t index;
>  
> -	dma_addr = gen_pool_alloc_owner(binding->chunk_pool, PAGE_SIZE,
> +	dma_addr = gen_pool_alloc_owner(binding->chunk_pool,
> +					1UL << binding->niov_shift,
>  					(void **)&owner);
>  	if (!dma_addr)
>  		return NULL;
>  
>  	offset = dma_addr - owner->base_dma_addr;
> -	index = offset / PAGE_SIZE;
> +	index = offset >> binding->niov_shift;

[Severity: Critical]
Does this bitwise shift cause a negative array index on 32-bit architectures?

Since a dma-buf scatterlist entry size (len) can be up to 4GB-1, the offset 
within a chunk can be >= 2GB. On 32-bit architectures, ssize_t is a 32-bit 
signed integer, so an offset >= 2GB overflows and becomes negative.

In the original code, the division promoted offset to an unsigned long 
because PAGE_SIZE is unsigned, resulting in a correct positive index:

    index = offset / PAGE_SIZE;

However, the type of a bitwise shift result is determined solely by the 
promoted left operand. By changing this to a bitwise right shift:

    index = offset >> binding->niov_shift;

the result remains signed. An arithmetic right shift of a negative number 
produces a negative index, which can lead to an out-of-bounds write on 
owner->area.niovs[index] later in the function.

>  	niov = &owner->area.niovs[index];
>  
>  	niov->desc.pp_magic = 0;

[ ... ]

> @@ -273,9 +277,12 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
>  		size_t len = sg_dma_len(sg);
>  		struct net_iov *niov;
>  
> -		if (!IS_ALIGNED(len, PAGE_SIZE)) {
> +		if (!IS_ALIGNED(dma_addr, niov_size) ||
> +		    !IS_ALIGNED(len, niov_size)) {
>  			err = -EINVAL;
> -			NL_SET_ERR_MSG(extack, "dma-buf SG length must be PAGE_SIZE aligned");
> +			NL_SET_ERR_MSG_FMT(extack,
> +					   "dmabuf sg entry (addr=%pad, len=%zu) not aligned to niov size %zu",
> +					   &dma_addr, len, niov_size);
>  			goto err_free_chunks;

[Severity: High]
This is a pre-existing issue, but can this error path lead to a panic by 
sleeping in atomic context?

When goto err_free_chunks is taken, the error path handles cleanup using 
gen_pool_for_each_chunk(), which iterates over chunks with rcu_read_lock() 
held:

lib/genalloc.c:gen_pool_for_each_chunk() {
    rcu_read_lock();
    list_for_each_entry_rcu(chunk, &(pool)->chunks, next_chunk)
        func(pool, chunk, data);
    rcu_read_unlock();
}

The callback net_devmem_dmabuf_free_chunk_owner() then frees the memory:

net/core/devmem.c:net_devmem_dmabuf_free_chunk_owner() {
    ...
    kvfree(owner->area.niovs);
    ...
}

If the allocated array is large enough to use the vmalloc fallback, kvfree() 
internally invokes vfree(). Since vfree() is not allowed in atomic context 
and calls might_sleep(), won't this trigger a 'scheduling while atomic' bug 
when the RCU read-side critical section is held?

>  		}
>  

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-tcpdm-large-niovs-v3-0-a3b693e76fcb@meta.com?part=1