RE: [EXTERNAL] [PATCH] net: otx2: handle NULL returned by xdp_convert_buff_to_frame()

-----Original Message-----
From: Chenyuan Yang <redacted>
Sent: Sunday, July 27, 2025 1:51 AM
To: Paolo Abeni <pabeni@redhat.com>
Cc: Geethasowjanya Akula <gakula@marvell.com>; Sunil Kovvuri Goutham
[off-list ref]; Subbaraya Sundeep Bhatta
[off-list ref]; Hariprasad Kelam [off-list ref]; Bharat
Bhushan [off-list ref]; andrew+netdev@lunn.ch;
davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
ast@kernel.org; daniel@iogearbox.net; hawk@kernel.org;
john.fastabend@gmail.com; sdf@fomichev.me; Suman Ghosh
[off-list ref]; netdev@vger.kernel.org; bpf@vger.kernel.org;
zzjas98@gmail.com
Subject: Re: [EXTERNAL] [PATCH] net: otx2: handle NULL returned by
xdp_convert_buff_to_frame()

On Thu, Jul 24, 2025 at 3:11 AM Paolo Abeni [off-list ref] wrote:

quoted

On 7/23/25 5:36 AM, Geethasowjanya Akula wrote:

quoted

quoted

-----Original Message-----
From: Geethasowjanya Akula
Sent: Wednesday, July 23, 2025 8:59 AM
To: Chenyuan Yang <redacted>; Sunil Kovvuri Goutham
[off-list ref]; Subbaraya Sundeep Bhatta
[off-list ref]; Hariprasad Kelam [off-list ref];
Bharat Bhushan [off-list ref]; andrew+netdev@lunn.ch;
davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
pabeni@redhat.com; ast@kernel.org; daniel@iogearbox.net;
hawk@kernel.org; john.fastabend@gmail.com; sdf@fomichev.me; Suman
Ghosh [off-list ref]
Cc: netdev@vger.kernel.org; bpf@vger.kernel.org; zzjas98@gmail.com
Subject: RE: [EXTERNAL] [PATCH] net: otx2: handle NULL returned by
xdp_convert_buff_to_frame()

quoted

-----Original Message-----
From: Chenyuan Yang <redacted>
Sent: Wednesday, July 23, 2025 6:03 AM
To: Sunil Kovvuri Goutham <sgoutham@marvell.com>; Geethasowjanya

Akula

quoted

[off-list ref]; Subbaraya Sundeep Bhatta
[off-list ref]; Hariprasad Kelam [off-list ref];
Bharat Bhushan [off-list ref]; andrew+netdev@lunn.ch;

davem@davemloft.net;

quoted

edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
ast@kernel.org; daniel@iogearbox.net; hawk@kernel.org;
john.fastabend@gmail.com; sdf@fomichev.me; Suman Ghosh
[off-list ref]
Cc: netdev@vger.kernel.org; bpf@vger.kernel.org;
zzjas98@gmail.com; Chenyuan Yang [off-list ref]
Subject: [EXTERNAL] [PATCH] net: otx2: handle NULL returned by
xdp_convert_buff_to_frame()

The xdp_convert_buff_to_frame() function can return NULL when
there is insufficient headroom in the buffer to store the
xdp_frame structure or when the driver didn't reserve enough tailroom

for skb_shared_info.

quoted

quoted

quoted

quoted

Currently, the otx2 driver does not check for this NULL return
value in two critical paths within otx2_xdp_rcv_pkt_handler():

1. XDP_TX case: Passes potentially NULL xdpf to
otx2_xdp_sq_append_pkt()

2.

quoted

XDP_REDIRECT error path: Calls xdp_return_frame() with potentially
NULL

This can lead to kernel crashes due to NULL pointer dereference.

Fix by adding proper NULL checks in both paths. For XDP_TX, return
false to indicate packet should be dropped. For XDP_REDIRECT error
path, only call
xdp_return_frame() if conversion succeeded, otherwise manually
free the page.

Please correct me if any error path is incorrect.

This is similar to the commit cc3628dcd851
("xen-netfront: handle NULL returned by xdp_convert_buff_to_frame()").

Signed-off-by: Chenyuan Yang <redacted>
Fixes: 94c80f748873 ("octeontx2-pf: use xdp_return_frame() to free
xdp
buffers")
---
drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c | 8
+++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git
a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
index 99ace381cc78..0c4c050b174a 100644

--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c

@@ -1534,6 +1534,9 @@ static bool otx2_xdp_rcv_pkt_handler(struct

otx2_nic *pfvf,
            qidx += pfvf->hw.tx_queues;
            cq->pool_ptrs++;
            xdpf = xdp_convert_buff_to_frame(&xdp);
+           if (unlikely(!xdpf))
+                   return false;
+
            return otx2_xdp_sq_append_pkt(pfvf, xdpf,
                                          cqe->sg.seg_addr,
                                          cqe->sg.seg_size, @@
-1558,7 +1561,10 @@ static bool otx2_xdp_rcv_pkt_handler(struct
otx2_nic *pfvf,
            otx2_dma_unmap_page(pfvf, iova, pfvf->rbsize,
                                DMA_FROM_DEVICE);
            xdpf = xdp_convert_buff_to_frame(&xdp);
-           xdp_return_frame(xdpf);
+           if (likely(xdpf))
+                   xdp_return_frame(xdpf);
+           else
+                   put_page(page);

Thanks for the fix. Given that the page is already freed, returning
true in this case makes sense.

This change might not be directly related to the current patch,
though. You can either include it here or we can submit a follow-up patch

to address it.

quoted

If I read correctly, returning false as the current patch is doing,
will make the later code in otx2_rcv_pkt_handler() unconditionally use
the just freed page.

I think returning true after put_page() is strictly necessary.

Thanks for the review and for catching that issue. You're right, returning false
would cause a use-after-free, as the caller would proceed to use the already
freed page.

I've updated the patch to return true in the XDP_TX failure case. I also adjusted

Returning true on XDP_TX failure requires the page to be freed. Otherwise, it should 
return false so the packet can be handled by otx2_rcv_pkt_handler.