>-----Original Message-----
>From: Chenyuan Yang <chenyuan0y@xxxxxxxxx>
>Sent: Wednesday, July 23, 2025 6:03 AM
>To: Sunil Kovvuri Goutham <sgoutham@xxxxxxxxxxx>; Geethasowjanya Akula
><gakula@xxxxxxxxxxx>; Subbaraya Sundeep Bhatta <sbhatta@xxxxxxxxxxx>;
>Hariprasad Kelam <hkelam@xxxxxxxxxxx>; Bharat Bhushan
><bbhushan2@xxxxxxxxxxx>; andrew+netdev@xxxxxxx;
>davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
>pabeni@xxxxxxxxxx; ast@xxxxxxxxxx; daniel@xxxxxxxxxxxxx;
>hawk@xxxxxxxxxx; john.fastabend@xxxxxxxxx; sdf@xxxxxxxxxxx; Suman
>Ghosh <sumang@xxxxxxxxxxx>
>Cc: netdev@xxxxxxxxxxxxxxx; bpf@xxxxxxxxxxxxxxx; zzjas98@xxxxxxxxx;
>Chenyuan Yang <chenyuan0y@xxxxxxxxx>
>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.
>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 <chenyuan0y@xxxxxxxxx>
>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.
> break;
> default:
> bpf_warn_invalid_xdp_action(pfvf->netdev, prog, act);
>--
>2.34.1
