* [PATCH] net: otx2: handle NULL returned by xdp_convert_buff_to_frame()
@ 2025-07-23 0:32 Chenyuan Yang
2025-07-23 3:29 ` [EXTERNAL] " Geethasowjanya Akula
0 siblings, 1 reply; 6+ messages in thread
From: Chenyuan Yang @ 2025-07-23 0:32 UTC (permalink / raw)
To: sgoutham, gakula, sbhatta, hkelam, bbhushan2, andrew+netdev,
davem, edumazet, kuba, pabeni, ast, daniel, hawk, john.fastabend,
sdf, sumang
Cc: netdev, bpf, zzjas98, Chenyuan Yang
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@gmail.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);
break;
default:
bpf_warn_invalid_xdp_action(pfvf->netdev, prog, act);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [EXTERNAL] [PATCH] net: otx2: handle NULL returned by xdp_convert_buff_to_frame()
2025-07-23 0:32 [PATCH] net: otx2: handle NULL returned by xdp_convert_buff_to_frame() Chenyuan Yang
@ 2025-07-23 3:29 ` Geethasowjanya Akula
2025-07-23 3:36 ` Geethasowjanya Akula
0 siblings, 1 reply; 6+ messages in thread
From: Geethasowjanya Akula @ 2025-07-23 3:29 UTC (permalink / raw)
To: Chenyuan Yang, Sunil Kovvuri Goutham, Subbaraya Sundeep Bhatta,
Hariprasad Kelam, Bharat Bhushan, 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
Cc: netdev@vger.kernel.org, bpf@vger.kernel.org, zzjas98@gmail.com
>-----Original Message-----
>From: Chenyuan Yang <chenyuan0y@gmail.com>
>Sent: Wednesday, July 23, 2025 6:03 AM
>To: Sunil Kovvuri Goutham <sgoutham@marvell.com>; Geethasowjanya Akula
><gakula@marvell.com>; Subbaraya Sundeep Bhatta <sbhatta@marvell.com>;
>Hariprasad Kelam <hkelam@marvell.com>; Bharat Bhushan
><bbhushan2@marvell.com>; 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 <sumang@marvell.com>
>Cc: netdev@vger.kernel.org; bpf@vger.kernel.org; zzjas98@gmail.com;
>Chenyuan Yang <chenyuan0y@gmail.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@gmail.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
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXTERNAL] [PATCH] net: otx2: handle NULL returned by xdp_convert_buff_to_frame()
2025-07-23 3:29 ` [EXTERNAL] " Geethasowjanya Akula
@ 2025-07-23 3:36 ` Geethasowjanya Akula
2025-07-24 10:11 ` Paolo Abeni
0 siblings, 1 reply; 6+ messages in thread
From: Geethasowjanya Akula @ 2025-07-23 3:36 UTC (permalink / raw)
To: Chenyuan Yang, Sunil Kovvuri Goutham, Subbaraya Sundeep Bhatta,
Hariprasad Kelam, Bharat Bhushan, 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
Cc: netdev@vger.kernel.org, bpf@vger.kernel.org, zzjas98@gmail.com
>-----Original Message-----
>From: Geethasowjanya Akula
>Sent: Wednesday, July 23, 2025 8:59 AM
>To: Chenyuan Yang <chenyuan0y@gmail.com>; Sunil Kovvuri Goutham
><sgoutham@marvell.com>; Subbaraya Sundeep Bhatta
><sbhatta@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>; Bharat
>Bhushan <bbhushan2@marvell.com>; 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 <sumang@marvell.com>
>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()
>
>
>
>>-----Original Message-----
>>From: Chenyuan Yang <chenyuan0y@gmail.com>
>>Sent: Wednesday, July 23, 2025 6:03 AM
>>To: Sunil Kovvuri Goutham <sgoutham@marvell.com>; Geethasowjanya
>Akula
>><gakula@marvell.com>; Subbaraya Sundeep Bhatta <sbhatta@marvell.com>;
>>Hariprasad Kelam <hkelam@marvell.com>; Bharat Bhushan
>><bbhushan2@marvell.com>; 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
>><sumang@marvell.com>
>>Cc: netdev@vger.kernel.org; bpf@vger.kernel.org; zzjas98@gmail.com;
>>Chenyuan Yang <chenyuan0y@gmail.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@gmail.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.
>> break;
>> default:
>> bpf_warn_invalid_xdp_action(pfvf->netdev, prog, act);
>>--
>>2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXTERNAL] [PATCH] net: otx2: handle NULL returned by xdp_convert_buff_to_frame()
2025-07-23 3:36 ` Geethasowjanya Akula
@ 2025-07-24 10:11 ` Paolo Abeni
2025-07-26 20:21 ` Chenyuan Yang
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2025-07-24 10:11 UTC (permalink / raw)
To: Geethasowjanya Akula, Chenyuan Yang, Sunil Kovvuri Goutham,
Subbaraya Sundeep Bhatta, Hariprasad Kelam, Bharat Bhushan,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, ast@kernel.org, daniel@iogearbox.net,
hawk@kernel.org, john.fastabend@gmail.com, sdf@fomichev.me,
Suman Ghosh
Cc: netdev@vger.kernel.org, bpf@vger.kernel.org, zzjas98@gmail.com
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@gmail.com>; Sunil Kovvuri Goutham
>> <sgoutham@marvell.com>; Subbaraya Sundeep Bhatta
>> <sbhatta@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>; Bharat
>> Bhushan <bbhushan2@marvell.com>; 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 <sumang@marvell.com>
>> 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()
>>
>>
>>
>>> -----Original Message-----
>>> From: Chenyuan Yang <chenyuan0y@gmail.com>
>>> Sent: Wednesday, July 23, 2025 6:03 AM
>>> To: Sunil Kovvuri Goutham <sgoutham@marvell.com>; Geethasowjanya
>> Akula
>>> <gakula@marvell.com>; Subbaraya Sundeep Bhatta <sbhatta@marvell.com>;
>>> Hariprasad Kelam <hkelam@marvell.com>; Bharat Bhushan
>>> <bbhushan2@marvell.com>; 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
>>> <sumang@marvell.com>
>>> Cc: netdev@vger.kernel.org; bpf@vger.kernel.org; zzjas98@gmail.com;
>>> Chenyuan Yang <chenyuan0y@gmail.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@gmail.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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXTERNAL] [PATCH] net: otx2: handle NULL returned by xdp_convert_buff_to_frame()
2025-07-24 10:11 ` Paolo Abeni
@ 2025-07-26 20:21 ` Chenyuan Yang
2025-07-28 8:38 ` Geethasowjanya Akula
0 siblings, 1 reply; 6+ messages in thread
From: Chenyuan Yang @ 2025-07-26 20:21 UTC (permalink / raw)
To: Paolo Abeni
Cc: Geethasowjanya Akula, Sunil Kovvuri Goutham,
Subbaraya Sundeep Bhatta, Hariprasad Kelam, Bharat Bhushan,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, ast@kernel.org, daniel@iogearbox.net,
hawk@kernel.org, john.fastabend@gmail.com, sdf@fomichev.me,
Suman Ghosh, netdev@vger.kernel.org, bpf@vger.kernel.org,
zzjas98@gmail.com
On Thu, Jul 24, 2025 at 3:11 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> 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@gmail.com>; Sunil Kovvuri Goutham
> >> <sgoutham@marvell.com>; Subbaraya Sundeep Bhatta
> >> <sbhatta@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>; Bharat
> >> Bhushan <bbhushan2@marvell.com>; 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 <sumang@marvell.com>
> >> 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()
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Chenyuan Yang <chenyuan0y@gmail.com>
> >>> Sent: Wednesday, July 23, 2025 6:03 AM
> >>> To: Sunil Kovvuri Goutham <sgoutham@marvell.com>; Geethasowjanya
> >> Akula
> >>> <gakula@marvell.com>; Subbaraya Sundeep Bhatta <sbhatta@marvell.com>;
> >>> Hariprasad Kelam <hkelam@marvell.com>; Bharat Bhushan
> >>> <bbhushan2@marvell.com>; 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
> >>> <sumang@marvell.com>
> >>> Cc: netdev@vger.kernel.org; bpf@vger.kernel.org; zzjas98@gmail.com;
> >>> Chenyuan Yang <chenyuan0y@gmail.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@gmail.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.
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
>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [EXTERNAL] [PATCH] net: otx2: handle NULL returned by xdp_convert_buff_to_frame()
2025-07-26 20:21 ` Chenyuan Yang
@ 2025-07-28 8:38 ` Geethasowjanya Akula
0 siblings, 0 replies; 6+ messages in thread
From: Geethasowjanya Akula @ 2025-07-28 8:38 UTC (permalink / raw)
To: Chenyuan Yang, Paolo Abeni
Cc: Sunil Kovvuri Goutham, Subbaraya Sundeep Bhatta, Hariprasad Kelam,
Bharat Bhushan, andrew+netdev@lunn.ch, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, ast@kernel.org,
daniel@iogearbox.net, hawk@kernel.org, john.fastabend@gmail.com,
sdf@fomichev.me, Suman Ghosh, netdev@vger.kernel.org,
bpf@vger.kernel.org, zzjas98@gmail.com
>-----Original Message-----
>From: Chenyuan Yang <chenyuan0y@gmail.com>
>Sent: Sunday, July 27, 2025 1:51 AM
>To: Paolo Abeni <pabeni@redhat.com>
>Cc: Geethasowjanya Akula <gakula@marvell.com>; Sunil Kovvuri Goutham
><sgoutham@marvell.com>; Subbaraya Sundeep Bhatta
><sbhatta@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>; Bharat
>Bhushan <bbhushan2@marvell.com>; andrew+netdev@lunn.ch;
>davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
>ast@kernel.org; daniel@iogearbox.net; hawk@kernel.org;
>john.fastabend@gmail.com; sdf@fomichev.me; Suman Ghosh
><sumang@marvell.com>; 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()
>
>On Thu, Jul 24, 2025 at 3:11 AM Paolo Abeni <pabeni@redhat.com> wrote:
>>
>> 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@gmail.com>; Sunil Kovvuri Goutham
>> >> <sgoutham@marvell.com>; Subbaraya Sundeep Bhatta
>> >> <sbhatta@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>;
>> >> Bharat Bhushan <bbhushan2@marvell.com>; 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 <sumang@marvell.com>
>> >> 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()
>> >>
>> >>
>> >>
>> >>> -----Original Message-----
>> >>> From: Chenyuan Yang <chenyuan0y@gmail.com>
>> >>> Sent: Wednesday, July 23, 2025 6:03 AM
>> >>> To: Sunil Kovvuri Goutham <sgoutham@marvell.com>; Geethasowjanya
>> >> Akula
>> >>> <gakula@marvell.com>; Subbaraya Sundeep Bhatta
>> >>> <sbhatta@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>;
>> >>> Bharat Bhushan <bbhushan2@marvell.com>; 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
>> >>> <sumang@marvell.com>
>> >>> Cc: netdev@vger.kernel.org; bpf@vger.kernel.org;
>> >>> zzjas98@gmail.com; Chenyuan Yang <chenyuan0y@gmail.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@gmail.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.
>
>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
Returning true on XDP_TX failure requires the page to be freed. Otherwise, it should
return false so the packet can be handled by otx2_rcv_pkt_handler.
>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
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-07-28 8:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-23 0:32 [PATCH] net: otx2: handle NULL returned by xdp_convert_buff_to_frame() Chenyuan Yang
2025-07-23 3:29 ` [EXTERNAL] " Geethasowjanya Akula
2025-07-23 3:36 ` Geethasowjanya Akula
2025-07-24 10:11 ` Paolo Abeni
2025-07-26 20:21 ` Chenyuan Yang
2025-07-28 8:38 ` Geethasowjanya Akula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).