Re: [PATCH net-next v14 4/5] ptr_ring: move free-space check into separate helper

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but `__ptr_ring_full()` lacks a check for a zero-sized ring, leading to a kernel crash (page fault/panic) when it dereferences `ZERO_SIZE_PTR`.
--

--- Patch [4]: [PATCH net-next v14 4/5] ptr_ring: move free-space check into separate helper ---
commit 883e8b9b82d44bf10bcbbae97968cf7b37d933bd
Author: Simon Schippers <[email protected]>

ptr_ring: move free-space check into separate helper

This commit moves the available free space check for a new entry into a
separate helper function. It allows callers to determine in advance whether a
subsequent produce call will succeed. Existing callers checking for a
non-zero return value remain unaffected.

> diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
> index d2c3629bbe451..631c43fde4407 100644
> --- a/include/linux/ptr_ring.h
> +++ b/include/linux/ptr_ring.h
> @@ -96,6 +96,26 @@ static inline bool ptr_ring_full_bh(struct ptr_ring *r)
>  	return ret;
>  }
>  
> +/* Report whether the next __ptr_ring_produce() has room for one entry:
> + * 0 means the single slot at r->queue[r->producer] is free, -ENOSPC means
> + * the ring is full, which is transient, and -EINVAL means r->size is 0,
> + * which is permanent. A caller that stops producing and waits for space
> + * must therefore do so only for -ENOSPC.
> + *
> + * Note: callers invoking this in a loop must use a compiler barrier,
> + * for example cpu_relax(). Callers must hold producer_lock.
> + */
> +static inline int __ptr_ring_check_produce(struct ptr_ring *r)
> +{
> +	if (unlikely(!r->size))
> +		return -EINVAL;
> +
> +	if (data_race(r->queue[r->producer]))
> +		return -ENOSPC;

[Severity: High]
This isn't a bug introduced by this patch, but __ptr_ring_full() in
include/linux/ptr_ring.h lacks a check for a zero-sized ring, unlike the
check added here.

include/linux/ptr_ring.h:__ptr_ring_full() {
	return data_race(r->queue[r->producer]);
}

Can this lead to a kernel Oops when it unconditionally dereferences r->queue?

When a ptr_ring is initialized or resized to a size of 0, r->queue becomes
ZERO_SIZE_PTR. Calling __ptr_ring_full() evaluates r->queue[r->producer]
unconditionally without checking if r->size == 0, leading to a page fault
when it dereferences ZERO_SIZE_PTR.

Would it make sense to update __ptr_ring_full() to safely check for
!r->size as well?

> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4