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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>-----Original Message-----
>From: Geethasowjanya Akula
>Sent: Wednesday, July 23, 2025 8:59 AM
>To: Chenyuan Yang <chenyuan0y@xxxxxxxxx>; Sunil Kovvuri Goutham
><sgoutham@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
>Subject: RE: [EXTERNAL] [PATCH] net: otx2: handle NULL returned by
>xdp_convert_buff_to_frame()
>
>
>
>>-----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.
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.
>> 		break;
>> 	default:
>> 		bpf_warn_invalid_xdp_action(pfvf->netdev, prog, act);
>>--
>>2.34.1






[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux