[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] xen-netfront: handle NULL returned by xdp_convert_buff_to_frame()
- To: Jürgen Groß <jgross@xxxxxxxx>, Jakub Kicinski <kuba@xxxxxxxxxx>
- From: Alexey <sdl@xxxxxxxx>
- Date: Thu, 17 Apr 2025 11:45:30 +0300
- Authentication-results: mail.nppct.ru (amavisd-new); dkim=pass (1024-bit key) reason="pass (just generated, assumed good)" header.d=nppct.ru
- Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx>, Eric Dumazet <edumazet@xxxxxxxxxx>, Paolo Abeni <pabeni@xxxxxxxxxx>, Alexei Starovoitov <ast@xxxxxxxxxx>, Daniel Borkmann <daniel@xxxxxxxxxxxxx>, Jesper Dangaard Brouer <hawk@xxxxxxxxxx>, John Fastabend <john.fastabend@xxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, netdev@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, bpf@xxxxxxxxxxxxxxx, lvc-project@xxxxxxxxxxxxxxxx, stable@xxxxxxxxxxxxxxx
- Delivery-date: Thu, 17 Apr 2025 08:45:49 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 17.04.2025 10:12, Jürgen Groß wrote:
On 17.04.25 09:00, Alexey wrote:
On 17.04.2025 03:58, Jakub Kicinski wrote:
On Mon, 14 Apr 2025 18:34:01 +0000 Alexey Nepomnyashih wrote:
get_page(pdata);
Please notice this get_page() here.
xdpf = xdp_convert_buff_to_frame(xdp);
+ if (unlikely(!xdpf)) {
+ trace_xdp_exception(queue->info->netdev, prog, act);
+ break;
+ }
Do you mean that it would be better to move the get_page(pdata) call
lower,
after checking for NULL in xdpf, so that the reference count is only
increased
after a successful conversion?
I think the error handling here is generally broken (or at least very
questionable).
I suspect that in case of at least some errors the get_page() is leaking
even without this new patch.
In case I'm wrong a comment reasoning why there is no leak should be
added.
Juergen
I think pdata is freed in xdp_return_frame_rx_napi() -> __xdp_return()
|
