netdev - RE: [EXTERNAL] [PATCH] net: otx2: handle NULL returned by xdp_convert_buff_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <CH0PR18MB43399E06C1EDC7DE70AE7170CD5FA@CH0PR18MB4339.namprd18.prod.outlook.com>
Date: Wed, 23 Jul 2025 03:29:04 +0000
From: Geethasowjanya Akula <gakula@...vell.com>
To: Chenyuan Yang <chenyuan0y@...il.com>,
        Sunil Kovvuri Goutham
	<sgoutham@...vell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@...vell.com>,
        Hariprasad Kelam <hkelam@...vell.com>,
        Bharat Bhushan
	<bbhushan2@...vell.com>,
        "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com"
	<edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "hawk@...nel.org"
	<hawk@...nel.org>,
        "john.fastabend@...il.com" <john.fastabend@...il.com>,
        "sdf@...ichev.me" <sdf@...ichev.me>, Suman Ghosh <sumang@...vell.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org"
	<bpf@...r.kernel.org>,
        "zzjas98@...il.com" <zzjas98@...il.com>
Subject: RE: [EXTERNAL] [PATCH] net: otx2: handle NULL returned by
 xdp_convert_buff_to_frame()



>-----Original Message-----
>From: Chenyuan Yang <chenyuan0y@...il.com>
>Sent: Wednesday, July 23, 2025 6:03 AM
>To: Sunil Kovvuri Goutham <sgoutham@...vell.com>; Geethasowjanya Akula
><gakula@...vell.com>; Subbaraya Sundeep Bhatta <sbhatta@...vell.com>;
>Hariprasad Kelam <hkelam@...vell.com>; Bharat Bhushan
><bbhushan2@...vell.com>; andrew+netdev@...n.ch;
>davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
>pabeni@...hat.com; ast@...nel.org; daniel@...earbox.net;
>hawk@...nel.org; john.fastabend@...il.com; sdf@...ichev.me; Suman
>Ghosh <sumang@...vell.com>
>Cc: netdev@...r.kernel.org; bpf@...r.kernel.org; zzjas98@...il.com;
>Chenyuan Yang <chenyuan0y@...il.com>
>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@...il.com>
>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