DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/xsc: fix use after free in some RXQ cleanup
@ 2025-09-09  7:04 David Marchand
  2025-09-09  8:08 ` Renyong Wan
  2025-09-09 14:45 ` David Marchand
  0 siblings, 2 replies; 5+ messages in thread
From: David Marchand @ 2025-09-09  7:04 UTC (permalink / raw)
  To: dev; +Cc: thomas, Renyong Wan, Na Na, Rong Qian, Xiaoxiong Zhang, Dongwei Xu

Debian 12 gcc complains about a use after free in this cleanup section.

[7/11] Compiling C object drivers/libtmp_rte_net_xsc.a.p/net_xsc_xsc_rx.c.o
In function 'xsc_rss_qp_create',
    inlined from 'xsc_rxq_rss_obj_new' at ../drivers/net/xsc/xsc_rx.c:565:8:
../drivers/net/xsc/xsc_rx.c:501:9: warning: pointer 'req' may be used after
	'free' [-Wuse-after-free]
  501 |         free(req);
      |         ^~~~~~~~~
../drivers/net/xsc/xsc_rx.c:501:9: note: call to 'free' here

Indeed, req may be free'd twice, as an error in the cleanup loop may
jump back to the set_qp_fail label.

Instead, skip the erroneous rxq and don't touch errno since all the code
jumping to set_qp_fail already sets it.

Fixes: 3991c890fb4c ("net/xsc: optimize RSS queue creation")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/xsc/xsc_rx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/xsc/xsc_rx.c b/drivers/net/xsc/xsc_rx.c
index 5f8003a1f6..5ff3f818c2 100644
--- a/drivers/net/xsc/xsc_rx.c
+++ b/drivers/net/xsc/xsc_rx.c
@@ -502,10 +502,8 @@ xsc_rss_qp_create(struct xsc_ethdev_priv *priv, int port_id)
 	for (i = 0; i < set_last_no; i++) {
 		xsc_unset_qp_info(xdev, rqn_base + i);
 		rxq_data = xsc_rxq_get(priv, i);
-		if (rxq_data == NULL) {
-			rte_errno = EINVAL;
-			goto set_qp_fail;
-		}
+		if (rxq_data == NULL)
+			continue;
 		rte_memzone_free(rxq_data->rq_pas);
 		rxq_data->rq_pas = NULL;
 	}
-- 
2.51.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/xsc: fix use after free in some RXQ cleanup
  2025-09-09  7:04 [PATCH] net/xsc: fix use after free in some RXQ cleanup David Marchand
@ 2025-09-09  8:08 ` Renyong Wan
  2025-09-09  9:22   ` Thomas Monjalon
  2025-09-09 14:45 ` David Marchand
  1 sibling, 1 reply; 5+ messages in thread
From: Renyong Wan @ 2025-09-09  8:08 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: thomas, Na Na, Rong Qian, Xiaoxiong Zhang, Dongwei Xu

Thanks David for catching this issue.
We'll address it in the 25.11 release.

On 2025/9/9 15:04, David Marchand wrote:
> Debian 12 gcc complains about a use after free in this cleanup section.
>
> [7/11] Compiling C object drivers/libtmp_rte_net_xsc.a.p/net_xsc_xsc_rx.c.o
> In function 'xsc_rss_qp_create',
>      inlined from 'xsc_rxq_rss_obj_new' at ../drivers/net/xsc/xsc_rx.c:565:8:
> ../drivers/net/xsc/xsc_rx.c:501:9: warning: pointer 'req' may be used after
> 	'free' [-Wuse-after-free]
>    501 |         free(req);
>        |         ^~~~~~~~~
> ../drivers/net/xsc/xsc_rx.c:501:9: note: call to 'free' here
>
> Indeed, req may be free'd twice, as an error in the cleanup loop may
> jump back to the set_qp_fail label.
>
> Instead, skip the erroneous rxq and don't touch errno since all the code
> jumping to set_qp_fail already sets it.
>
> Fixes: 3991c890fb4c ("net/xsc: optimize RSS queue creation")
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>   drivers/net/xsc/xsc_rx.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/xsc/xsc_rx.c b/drivers/net/xsc/xsc_rx.c
> index 5f8003a1f6..5ff3f818c2 100644
> --- a/drivers/net/xsc/xsc_rx.c
> +++ b/drivers/net/xsc/xsc_rx.c
> @@ -502,10 +502,8 @@ xsc_rss_qp_create(struct xsc_ethdev_priv *priv, int port_id)
>   	for (i = 0; i < set_last_no; i++) {
>   		xsc_unset_qp_info(xdev, rqn_base + i);
>   		rxq_data = xsc_rxq_get(priv, i);
> -		if (rxq_data == NULL) {
> -			rte_errno = EINVAL;
> -			goto set_qp_fail;
> -		}
> +		if (rxq_data == NULL)
> +			continue;
>   		rte_memzone_free(rxq_data->rq_pas);
>   		rxq_data->rq_pas = NULL;
>   	}


-- 
Best regards,
Renyong Wan

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/xsc: fix use after free in some RXQ cleanup
  2025-09-09  8:08 ` Renyong Wan
@ 2025-09-09  9:22   ` Thomas Monjalon
  2025-09-09 13:04     ` Renyong Wan
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Monjalon @ 2025-09-09  9:22 UTC (permalink / raw)
  To: Renyong Wan, Na Na, Rong Qian, Xiaoxiong Zhang, Dongwei Xu
  Cc: David Marchand, dev

09/09/2025 10:08, Renyong Wan:
> Thanks David for catching this issue.
> We'll address it in the 25.11 release.

I don't understand your answer.
Do you ack this change?
We want to merge it today because it is breaking our CI on the main branch
(next-net has been pulled yesterday).


> On 2025/9/9 15:04, David Marchand wrote:
> > Debian 12 gcc complains about a use after free in this cleanup section.
> >
> > [7/11] Compiling C object drivers/libtmp_rte_net_xsc.a.p/net_xsc_xsc_rx.c.o
> > In function 'xsc_rss_qp_create',
> >      inlined from 'xsc_rxq_rss_obj_new' at ../drivers/net/xsc/xsc_rx.c:565:8:
> > ../drivers/net/xsc/xsc_rx.c:501:9: warning: pointer 'req' may be used after
> > 	'free' [-Wuse-after-free]
> >    501 |         free(req);
> >        |         ^~~~~~~~~
> > ../drivers/net/xsc/xsc_rx.c:501:9: note: call to 'free' here
> >
> > Indeed, req may be free'd twice, as an error in the cleanup loop may
> > jump back to the set_qp_fail label.
> >
> > Instead, skip the erroneous rxq and don't touch errno since all the code
> > jumping to set_qp_fail already sets it.
> >
> > Fixes: 3991c890fb4c ("net/xsc: optimize RSS queue creation")
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >   drivers/net/xsc/xsc_rx.c | 6 ++----
> >   1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/xsc/xsc_rx.c b/drivers/net/xsc/xsc_rx.c
> > index 5f8003a1f6..5ff3f818c2 100644
> > --- a/drivers/net/xsc/xsc_rx.c
> > +++ b/drivers/net/xsc/xsc_rx.c
> > @@ -502,10 +502,8 @@ xsc_rss_qp_create(struct xsc_ethdev_priv *priv, int port_id)
> >   	for (i = 0; i < set_last_no; i++) {
> >   		xsc_unset_qp_info(xdev, rqn_base + i);
> >   		rxq_data = xsc_rxq_get(priv, i);
> > -		if (rxq_data == NULL) {
> > -			rte_errno = EINVAL;
> > -			goto set_qp_fail;
> > -		}
> > +		if (rxq_data == NULL)
> > +			continue;
> >   		rte_memzone_free(rxq_data->rq_pas);
> >   		rxq_data->rq_pas = NULL;
> >   	}




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/xsc: fix use after free in some RXQ cleanup
  2025-09-09  9:22   ` Thomas Monjalon
@ 2025-09-09 13:04     ` Renyong Wan
  0 siblings, 0 replies; 5+ messages in thread
From: Renyong Wan @ 2025-09-09 13:04 UTC (permalink / raw)
  To: Thomas Monjalon, Na Na, Rong Qian, Xiaoxiong Zhang, Dongwei Xu
  Cc: David Marchand, dev

On 2025/9/9 17:22, Thomas Monjalon wrote:
> 09/09/2025 10:08, Renyong Wan:
>> Thanks David for catching this issue.
>> We'll address it in the 25.11 release.
> I don't understand your answer.
> Do you ack this change?
> We want to merge it today because it is breaking our CI on the main branch
> (next-net has been pulled yesterday).
Sorry, I did not carefully read the mail.
Thanks for the fix.

Acked-by: Renyong Wan <wanry@yunsilicon.com>

>
>> On 2025/9/9 15:04, David Marchand wrote:
>>> Debian 12 gcc complains about a use after free in this cleanup section.
>>>
>>> [7/11] Compiling C object drivers/libtmp_rte_net_xsc.a.p/net_xsc_xsc_rx.c.o
>>> In function 'xsc_rss_qp_create',
>>>       inlined from 'xsc_rxq_rss_obj_new' at ../drivers/net/xsc/xsc_rx.c:565:8:
>>> ../drivers/net/xsc/xsc_rx.c:501:9: warning: pointer 'req' may be used after
>>> 	'free' [-Wuse-after-free]
>>>     501 |         free(req);
>>>         |         ^~~~~~~~~
>>> ../drivers/net/xsc/xsc_rx.c:501:9: note: call to 'free' here
>>>
>>> Indeed, req may be free'd twice, as an error in the cleanup loop may
>>> jump back to the set_qp_fail label.
>>>
>>> Instead, skip the erroneous rxq and don't touch errno since all the code
>>> jumping to set_qp_fail already sets it.
>>>
>>> Fixes: 3991c890fb4c ("net/xsc: optimize RSS queue creation")
>>>
>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>> ---
>>>    drivers/net/xsc/xsc_rx.c | 6 ++----
>>>    1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/xsc/xsc_rx.c b/drivers/net/xsc/xsc_rx.c
>>> index 5f8003a1f6..5ff3f818c2 100644
>>> --- a/drivers/net/xsc/xsc_rx.c
>>> +++ b/drivers/net/xsc/xsc_rx.c
>>> @@ -502,10 +502,8 @@ xsc_rss_qp_create(struct xsc_ethdev_priv *priv, int port_id)
>>>    	for (i = 0; i < set_last_no; i++) {
>>>    		xsc_unset_qp_info(xdev, rqn_base + i);
>>>    		rxq_data = xsc_rxq_get(priv, i);
>>> -		if (rxq_data == NULL) {
>>> -			rte_errno = EINVAL;
>>> -			goto set_qp_fail;
>>> -		}
>>> +		if (rxq_data == NULL)
>>> +			continue;
>>>    		rte_memzone_free(rxq_data->rq_pas);
>>>    		rxq_data->rq_pas = NULL;
>>>    	}
>
>


-- 
Best regards,
Renyong Wan

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/xsc: fix use after free in some RXQ cleanup
  2025-09-09  7:04 [PATCH] net/xsc: fix use after free in some RXQ cleanup David Marchand
  2025-09-09  8:08 ` Renyong Wan
@ 2025-09-09 14:45 ` David Marchand
  1 sibling, 0 replies; 5+ messages in thread
From: David Marchand @ 2025-09-09 14:45 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, Renyong Wan, Na Na, Rong Qian, Xiaoxiong Zhang, Dongwei Xu

On Tue, 9 Sept 2025 at 09:05, David Marchand <david.marchand@redhat.com> wrote:
>
> Debian 12 gcc complains about a use after free in this cleanup section.
>
> [7/11] Compiling C object drivers/libtmp_rte_net_xsc.a.p/net_xsc_xsc_rx.c.o
> In function 'xsc_rss_qp_create',
>     inlined from 'xsc_rxq_rss_obj_new' at ../drivers/net/xsc/xsc_rx.c:565:8:
> ../drivers/net/xsc/xsc_rx.c:501:9: warning: pointer 'req' may be used after
>         'free' [-Wuse-after-free]
>   501 |         free(req);
>       |         ^~~~~~~~~
> ../drivers/net/xsc/xsc_rx.c:501:9: note: call to 'free' here
>
> Indeed, req may be free'd twice, as an error in the cleanup loop may
> jump back to the set_qp_fail label.
>
> Instead, skip the erroneous rxq and don't touch errno since all the code
> jumping to set_qp_fail already sets it.
>
> Fixes: 3991c890fb4c ("net/xsc: optimize RSS queue creation")
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Renyong Wan <wanry@yunsilicon.com>

Applied, thanks.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-09-09 14:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-09  7:04 [PATCH] net/xsc: fix use after free in some RXQ cleanup David Marchand
2025-09-09  8:08 ` Renyong Wan
2025-09-09  9:22   ` Thomas Monjalon
2025-09-09 13:04     ` Renyong Wan
2025-09-09 14:45 ` David Marchand

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