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:

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.

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.

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 the XDP_REDIRECT error path to do the same after calling
put_page(), preventing a fall-through.

Does the updated patch below look correct? If so, I'll send out a formal v2.

---
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c | 10 +++++++++-
 1 file changed, 9 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..4e1b9a3f6e51 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 true;
+
  return otx2_xdp_sq_append_pkt(pfvf, xdpf,
        cqe->sg.seg_addr,
        cqe->sg.seg_size,

@@ -1558,7 +1561,12 @@ 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);
+ return true;
+ }
  break;
  default:
  bpf_warn_invalid_xdp_action(pfvf->netdev, prog, act);
---

/P