On Mon, Dec 20, 2021 at 9:10 PM Ciara Loftus <ciara.loftus@xxxxxxxxx> wrote:
>
> This commit initialises the xskb's free_list_node when the xskb is
> allocated. This prevents a potential false negative returned from a call
> to list_empty for that node, such as the one introduced in commit
> 199d983bc015 ("xsk: Fix crash on double free in buffer pool")
>
> In my environment this issue caused packets to not be received by
> the xdpsock application if the traffic was running prior to application
> launch. This happened when the first batch of packets failed the xskmap
> lookup and XDP_PASS was returned from the bpf program. This action is
> handled in the i40e zc driver (and others) by allocating an skbuff,
> freeing the xdp_buff and adding the associated xskb to the
> xsk_buff_pool's free_list if it hadn't been added already. Without this
> fix, the xskb is not added to the free_list because the check to determine
> if it was added already returns an invalid positive result. Later, this
> caused allocation errors in the driver and the failure to receive packets.
Thank you for this fix Ciara! Though I do think the Fixes tag should
be the one above: 199d983bc015 ("xsk: Fix crash on double free in
buffer pool"). Before that commit, there was no test for an empty list
in the xp_free path. The entry was unconditionally put on the list and
"initialized" in that way, so that code will work without this patch.
What do you think?
Acked-by: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
> Fixes: 2b43470add8c ("xsk: Introduce AF_XDP buffer allocation API")
>
> Signed-off-by: Ciara Loftus <ciara.loftus@xxxxxxxxx>
> ---
> net/xdp/xsk_buff_pool.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index bc4ad48ea4f0..fd39bb660ebc 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -83,6 +83,7 @@ struct xsk_buff_pool *xp_create_and_assign_umem(struct xdp_sock *xs,
> xskb = &pool->heads[i];
> xskb->pool = pool;
> xskb->xdp.frame_sz = umem->chunk_size - umem->headroom;
> + INIT_LIST_HEAD(&xskb->free_list_node);
> if (pool->unaligned)
> pool->free_heads[i] = xskb;
> else
> --
> 2.17.1
>
