Re: [PATCH] net/gve: fix refill logic causing memory corruption

[Ferruh Yigit <ferruh.yigit@amd.com>]

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.guo@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Joshua Washington <joshwash@google.com>
> Reviewed-by: Rushil Gupta <rushilg@google.com>
> Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
>

Applied to dpdk-next-net/main, thanks.