Re: [PATCH net v3] xsk: fix NULL pointer dereference in __xsk_rcv()

[Cen Zhang (Microsoft)]

Hi Simon,

Thanks for the comments.

Given the extra complexity of reusing the pool-global xskb_list here,
including the locking/concurrency issues and the hidden implementation
assumptions needed to reuse the existing frag helpers, I would prefer to
go back to the v2 local-list implementation, which is correct, logically
simpler, and easier to maintain.

@Jason, regarding your v2 memory leak concern:

> It will cause a memory leak because the current xsk_xdp that is not
> added to the local list will miss the chance to get freed? And the
> empty list_node cannot be easily freed by xp_free()...

IIUC, it would not leak. The !list_empty(&xskb->list_node) case can only
happen when fresh aligned-mode allocation returns the same xskb for a
duplicated user Fill Ring address. In that case, the xskb has already
been added to the local staging list by an earlier iteration, so the
error path will walk that list, do list_del_init(), and then
xsk_buff_free() can recycle it. For buffers returned from the free_list,
in either aligned or unaligned mode, xsk_buff_alloc() already did
list_del_init(). For fresh unaligned-mode allocations, xskb metadata
comes from free_heads, so duplicated user addresses should not return the
same in-list xskb.

I'll prepare v4 based on the v2 local-list approach shortly.

Thanks,
Cen