netdev - Re: [PATCH bpf-next 4/4] selftests: xsk: destroy BPF resources only when ctx refcount drops to 0

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 30 Jun 2022 22:52:48 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Magnus Karlsson <magnus.karlsson@...il.com>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc:     bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii@...nel.org>,
        Network Development <netdev@...r.kernel.org>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>
Subject: Re: [PATCH bpf-next 4/4] selftests: xsk: destroy BPF resources only
 when ctx refcount drops to 0

On 6/30/22 11:58 AM, Magnus Karlsson wrote:
> On Wed, Jun 29, 2022 at 4:39 PM Maciej Fijalkowski
> <maciej.fijalkowski@...el.com> wrote:
>>
>> Currently, xsk_socket__delete frees BPF resources regardless of ctx
>> refcount. Xdpxceiver has a test to verify whether underlying BPF
>> resources would not be wiped out after closing XSK socket that was bound
>> to interface with other active sockets. From library's xsk part
>> perspective it also means that the internal xsk context is shared and
>> its refcount is bumped accordingly.
>>
>> After a switch to loading XDP prog based on previously opened XSK
>> socket, mentioned xdpxceiver test fails with:
>> not ok 16 [xdpxceiver.c:swap_xsk_resources:1334]: ERROR: 9/"Bad file descriptor
>>
>> which means that in swap_xsk_resources(), xsk_socket__delete() released
>> xskmap which in turn caused a failure of xsk_socket__update_xskmap().
>>
>> To fix this, when deleting socket, decrement ctx refcount before
>> releasing BPF resources and do so only when refcount dropped to 0 which
>> means there are no more active sockets for this ctx so BPF resources can
>> be freed safely.
> 
> Please fix this in libxdp too as the bug is present there also.
> 
> Acked-by: Magnus Karlsson <magnus.karlsson@...el.com>

[...]
>> @@ -1238,7 +1236,10 @@ void xsk_socket__delete(struct xsk_socket *xsk)
>>
>>          ctx = xsk->ctx;
>>          umem = ctx->umem;
>> -       if (ctx->prog_fd != -1) {
>> +
>> +       xsk_put_ctx(ctx, true);
>> +
>> +       if (!ctx->refcount) {
>>                  xsk_delete_bpf_maps(xsk);
>>                  close(ctx->prog_fd);
>>                  if (ctx->has_bpf_link)
>> @@ -1257,7 +1258,6 @@ void xsk_socket__delete(struct xsk_socket *xsk)
>>                  }
>>          }
>>
>> -       xsk_put_ctx(ctx, true);
>>
>>          umem->refcount--;

Applied & also fixed up the double newline. Thanks everyone!