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] [day] [month] [year] [list]
Message-ID: <77ce8301-38e5-4d13-9b76-0d731f8b6a7e@redhat.com>
Date: Thu, 24 Jul 2025 12:11:25 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Geethasowjanya Akula <gakula@...vell.com>,
 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>, "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()

On 7/23/25 5:36 AM, Geethasowjanya Akula wrote:
>> -----Original Message-----
>> From: Geethasowjanya Akula
>> Sent: Wednesday, July 23, 2025 8:59 AM
>> 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;
>> 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
>> 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.
> 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.

/P