On 10/4/2024 2:05 AM, Joshua Washington wrote:
> There is a seemingly mundane error in the RX refill path which can lead
> to major issues and ultimately program crashing.
>
> This error occurs as part of an edge case where the exact number of
> buffers the refill causes the ring to wrap around to 0. The current
> refill logic is split into two conditions: first, when the number of
> buffers to refill is greater than the number of buffers left in the ring
> before wraparound occurs; second, when the opposite is true, and there
> are enough buffers before wraparound to refill all buffers.
>
> In this edge case, the first condition erroneously uses a (<) condition
> to decide whether to wrap around, when it should have been (<=). In that
> case, the second condition would run and the tail pointer would be set
> to an invalid value (RING_SIZE). This causes a number of cascading
> failures.
>
> 1. The first issue rather mundane in that rxq->bufq_tail == RING_SIZE at
> the end of the refill, this will correct itself on the next refill
> without any sort of memory leak or courrption;
> 2. The second failure is that the head pointer would end up overrunning
> the tail because the last buffer that is refilled is refilled at
> sw_ring[RING_SIZE] instead of sw_ring[0]. This would cause the driver
> to give the application a stale mbuf, one that has been potentially
> freed or is otherwise stale;
> 3. The third failure comes from the fact that the software ring is being
> overrun. Because we directly use the sw_ring pointer to refill
> buffers, when sw_ring[RING_SIZE] is filled, a buffer overflow occurs.
> The overwritten data has the potential to be important data, and this
> can potentially cause the program to crash outright.
>
> This patch fixes the refill bug while greatly simplifying the logic so
> that it is much less error-prone.
>
> Fixes: 45da16b5b181 ("net/gve: support basic Rx data path for DQO")
> Cc: junfeng....@intel.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Joshua Washington <joshw...@google.com>
> Reviewed-by: Rushil Gupta <rush...@google.com>
> Reviewed-by: Praveen Kaligineedi <pkaligine...@google.com>
>
Applied to dpdk-next-net/main, thanks.
