On Wed, 28 Oct 2020 at 08:32, Magnus Karlsson <magnus.karlsson@xxxxxxxxx> wrote:
>
> From: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
>
> Fix a possible memory leak at xsk socket close that is caused by the
> refcounting of the umem object being wrong. The reference count of the
> umem was decremented only after the pool had been freed. Note that if
> the buffer pool is destroyed, it is important that the umem is
> destroyed after the pool, otherwise the umem would disappear while the
> driver is still running. And as the buffer pool needs to be destroyed
> in a work queue, the umem is also (if its refcount reaches zero)
> destroyed after the buffer pool in that same work queue.
>
> What was missing is that the refcount also needs to be decremented
> when the pool is not freed and when the pool has not even been
> created. The first case happens when the refcount of the pool is
> higher than 1, i.e. it is still being used by some other socket using
> the same device and queue id. In this case, it is safe to decrement
> the refcount of the umem outside of the work queue as the umem will
> never be freed because the refcount of the umem is always greater than
> or equal to the refcount of the buffer pool. The second case is if the
> buffer pool has not been created yet, i.e. the socket was closed
> before it was bound but after the umem was created. In this case, it
> is safe to destroy the umem outside of the work queue, since there is
> no pool that can use it by definition.
>
> Fixes: 1c1efc2af158 ("xsk: Create and free buffer pool independently from umem")
> Reported-by: syzbot+eb71df123dc2be2c1456@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
Acked-by: Björn Töpel <bjorn.topel@xxxxxxxxx>
