* [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).