Netdev List
 help / color / mirror / Atom feed
* [PATCH net] ice: fix memory leak in ice_lbtest_prepare_rings()
@ 2026-06-09 12:50 Dawei Feng
  2026-06-09 14:27 ` [Intel-wired-lan] " Marcin Szycik
  0 siblings, 1 reply; 5+ messages in thread
From: Dawei Feng @ 2026-06-09 12:50 UTC (permalink / raw)
  To: Tony Nguyen
  Cc: Przemek Kitszel, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, netdev,
	linux-kernel, jianhao.xu, Dawei Feng, stable, Zilin Guan

While ice_lbtest_prepare_rings() correctly frees Rx rings if
ice_vsi_start_all_rx_rings() fails, the earlier error paths for
ice_vsi_setup_rx_rings() and ice_vsi_cfg_lan() jump past this cleanup.
If Rx ring setup or LAN configuration fails, the function leaks the
initialized Rx resources.

Fix this by routing these earlier failures to the existing
err_start_rx_ring label. This ensures the Rx rings are properly freed
before tearing down the Tx state.

The bug was first flagged by an experimental analysis tool we are
developing for kernel memory-management bugs while analyzing
v6.13-rc1. The tool is still under development and is not yet publicly
available. Manual inspection confirms that the bug is still
present in v7.1-rc5.

An x86_64 allyesconfig build showed no new warnings. As we do not have an
Intel E800 Series adapter available to run the ethtool offline loopback
selftest, no runtime testing was able to be performed.

Fixes: 0e674aeb0b77 ("ice: Add handler for ethtool selftest")
Cc: stable@vger.kernel.org
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
---
 drivers/net/ethernet/intel/ice/ice_ethtool.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index f28416a707d7..7c81ca313645 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -1065,11 +1065,11 @@ static int ice_lbtest_prepare_rings(struct ice_vsi *vsi)
 
 	status = ice_vsi_setup_rx_rings(vsi);
 	if (status)
-		goto err_setup_rx_ring;
+		goto err_start_rx_ring;
 
 	status = ice_vsi_cfg_lan(vsi);
 	if (status)
-		goto err_setup_rx_ring;
+		goto err_start_rx_ring;
 
 	status = ice_vsi_start_all_rx_rings(vsi);
 	if (status)
@@ -1079,7 +1079,6 @@ static int ice_lbtest_prepare_rings(struct ice_vsi *vsi)
 
 err_start_rx_ring:
 	ice_vsi_free_rx_rings(vsi);
-err_setup_rx_ring:
 	ice_vsi_stop_lan_tx_rings(vsi, ICE_NO_RESET, 0);
 err_setup_tx_ring:
 	ice_vsi_free_tx_rings(vsi);
-- 
2.34.1


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

* Re: [Intel-wired-lan] [PATCH net] ice: fix memory leak in ice_lbtest_prepare_rings()
  2026-06-09 12:50 [PATCH net] ice: fix memory leak in ice_lbtest_prepare_rings() Dawei Feng
@ 2026-06-09 14:27 ` Marcin Szycik
  2026-06-09 22:27   ` Jacob Keller
  0 siblings, 1 reply; 5+ messages in thread
From: Marcin Szycik @ 2026-06-09 14:27 UTC (permalink / raw)
  To: Dawei Feng, Tony Nguyen
  Cc: Przemek Kitszel, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, netdev,
	linux-kernel, jianhao.xu, stable, Zilin Guan



On 09.06.2026 14:50, Dawei Feng wrote:
> While ice_lbtest_prepare_rings() correctly frees Rx rings if
> ice_vsi_start_all_rx_rings() fails, the earlier error paths for
> ice_vsi_setup_rx_rings() and ice_vsi_cfg_lan() jump past this cleanup.
> If Rx ring setup or LAN configuration fails, the function leaks the
> initialized Rx resources.
> 
> Fix this by routing these earlier failures to the existing
> err_start_rx_ring label. This ensures the Rx rings are properly freed
> before tearing down the Tx state.
> 
> The bug was first flagged by an experimental analysis tool we are
> developing for kernel memory-management bugs while analyzing
> v6.13-rc1. The tool is still under development and is not yet publicly
> available. Manual inspection confirms that the bug is still
> present in v7.1-rc5.
> 
> An x86_64 allyesconfig build showed no new warnings. As we do not have an
> Intel E800 Series adapter available to run the ethtool offline loopback
> selftest, no runtime testing was able to be performed.

IMO last two paragraphs should not be included in commit message,
rather after ---.

> Fixes: 0e674aeb0b77 ("ice: Add handler for ethtool selftest")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> ---
>  drivers/net/ethernet/intel/ice/ice_ethtool.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index f28416a707d7..7c81ca313645 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -1065,11 +1065,11 @@ static int ice_lbtest_prepare_rings(struct ice_vsi *vsi)
>  
>  	status = ice_vsi_setup_rx_rings(vsi);
>  	if (status)
> -		goto err_setup_rx_ring;
> +		goto err_start_rx_ring;
>  
>  	status = ice_vsi_cfg_lan(vsi);
>  	if (status)
> -		goto err_setup_rx_ring;
> +		goto err_start_rx_ring;
>  
>  	status = ice_vsi_start_all_rx_rings(vsi);
>  	if (status)
> @@ -1079,7 +1079,6 @@ static int ice_lbtest_prepare_rings(struct ice_vsi *vsi)
>  
>  err_start_rx_ring:
>  	ice_vsi_free_rx_rings(vsi);
> -err_setup_rx_ring:
>  	ice_vsi_stop_lan_tx_rings(vsi, ICE_NO_RESET, 0);

Correct me if I'm wrong, but looks like unroll order is reversed:
ice_vsi_stop_lan_tx_rings() unrolls ice_vsi_cfg_lan()
ice_vsi_free_rx_rings() unrolls ice_vsi_setup_rx_rings()
(was reversed before this patch too, but since we're fixing it, might as well)

>  err_setup_tx_ring:
>  	ice_vsi_free_tx_rings(vsi);

Thanks,
Marcin

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

* Re: [Intel-wired-lan] [PATCH net] ice: fix memory leak in ice_lbtest_prepare_rings()
  2026-06-09 14:27 ` [Intel-wired-lan] " Marcin Szycik
@ 2026-06-09 22:27   ` Jacob Keller
  2026-06-11  2:02     ` Dawei Feng
  0 siblings, 1 reply; 5+ messages in thread
From: Jacob Keller @ 2026-06-09 22:27 UTC (permalink / raw)
  To: Marcin Szycik, Dawei Feng, Tony Nguyen
  Cc: Przemek Kitszel, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, netdev,
	linux-kernel, jianhao.xu, stable, Zilin Guan

On 6/9/2026 7:27 AM, Marcin Szycik wrote:
> 
> 
> On 09.06.2026 14:50, Dawei Feng wrote:
>> While ice_lbtest_prepare_rings() correctly frees Rx rings if
>> ice_vsi_start_all_rx_rings() fails, the earlier error paths for
>> ice_vsi_setup_rx_rings() and ice_vsi_cfg_lan() jump past this cleanup.
>> If Rx ring setup or LAN configuration fails, the function leaks the
>> initialized Rx resources.
>>
>> Fix this by routing these earlier failures to the existing
>> err_start_rx_ring label. This ensures the Rx rings are properly freed
>> before tearing down the Tx state.
>>
>> The bug was first flagged by an experimental analysis tool we are
>> developing for kernel memory-management bugs while analyzing
>> v6.13-rc1. The tool is still under development and is not yet publicly
>> available. Manual inspection confirms that the bug is still
>> present in v7.1-rc5.
>>
>> An x86_64 allyesconfig build showed no new warnings. As we do not have an
>> Intel E800 Series adapter available to run the ethtool offline loopback
>> selftest, no runtime testing was able to be performed.
> 
> IMO last two paragraphs should not be included in commit message,
> rather after ---.
> 

If this gets queued up by Tony it will get testing by Intel's validation
team ya.

>> Fixes: 0e674aeb0b77 ("ice: Add handler for ethtool selftest")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
>> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
>> ---
>>  drivers/net/ethernet/intel/ice/ice_ethtool.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
>> index f28416a707d7..7c81ca313645 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
>> @@ -1065,11 +1065,11 @@ static int ice_lbtest_prepare_rings(struct ice_vsi *vsi)
>>  
>>  	status = ice_vsi_setup_rx_rings(vsi);
>>  	if (status)
>> -		goto err_setup_rx_ring;
>> +		goto err_start_rx_ring;
>>  
>>  	status = ice_vsi_cfg_lan(vsi);
>>  	if (status)
>> -		goto err_setup_rx_ring;
>> +		goto err_start_rx_ring;
>>  
>>  	status = ice_vsi_start_all_rx_rings(vsi);
>>  	if (status)
>> @@ -1079,7 +1079,6 @@ static int ice_lbtest_prepare_rings(struct ice_vsi *vsi)
>>  
>>  err_start_rx_ring:
>>  	ice_vsi_free_rx_rings(vsi);
>> -err_setup_rx_ring:
>>  	ice_vsi_stop_lan_tx_rings(vsi, ICE_NO_RESET, 0);
> 
> Correct me if I'm wrong, but looks like unroll order is reversed:
> ice_vsi_stop_lan_tx_rings() unrolls ice_vsi_cfg_lan()
> ice_vsi_free_rx_rings() unrolls ice_vsi_setup_rx_rings()
> (was reversed before this patch too, but since we're fixing it, might as well)
> 
>>  err_setup_tx_ring:
>>  	ice_vsi_free_tx_rings(vsi);
> 
> Thanks,
> Marcin


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

* Re: [Intel-wired-lan] [PATCH net] ice: fix memory leak in ice_lbtest_prepare_rings()
  2026-06-09 22:27   ` Jacob Keller
@ 2026-06-11  2:02     ` Dawei Feng
  2026-06-11  9:57       ` Marcin Szycik
  0 siblings, 1 reply; 5+ messages in thread
From: Dawei Feng @ 2026-06-11  2:02 UTC (permalink / raw)
  To: jacob.e.keller
  Cc: andrew+netdev, anthony.l.nguyen, davem, dawei.feng, edumazet,
	intel-wired-lan, jianhao.xu, kuba, linux-kernel, marcin.szycik,
	netdev, pabeni, przemyslaw.kitszel, stable, zilin

Hi Marcin,

Thanks for your review.

On Tue, 9 Jun 2026 at 16:27:20 Marcin Szycik wrote:
> IMO last two paragraphs should not be included in commit message,
> rather after ---.

The reason the manual inspection and testing commentary was placed above
the `---` line is that we were strictly following the example template
provided in Documentation/process/researcher-guidelines.rst. 

In the researcher-guidelines[1], the example explicitly places the build
and hardware testing disclaimer before the Signed-off-by tags, which is
why we included it directly in the commit message.

Please let me know if you would like a v2 to adjust the position of the
mentioned commit log details.

> Correct me if I'm wrong, but looks like unroll order is reversed:
> ice_vsi_stop_lan_tx_rings() unrolls ice_vsi_cfg_lan()
> ice_vsi_free_rx_rings() unrolls ice_vsi_setup_rx_rings()
> (was reversed before this patch too, but since we're fixing it, might as well)

You are right. I'll update it in v2.

[1] https://docs.kernel.org/process/researcher-guidelines.html

Best regards,
Dawei

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

* Re: [Intel-wired-lan] [PATCH net] ice: fix memory leak in ice_lbtest_prepare_rings()
  2026-06-11  2:02     ` Dawei Feng
@ 2026-06-11  9:57       ` Marcin Szycik
  0 siblings, 0 replies; 5+ messages in thread
From: Marcin Szycik @ 2026-06-11  9:57 UTC (permalink / raw)
  To: Dawei Feng, jacob.e.keller
  Cc: andrew+netdev, anthony.l.nguyen, davem, edumazet, intel-wired-lan,
	jianhao.xu, kuba, linux-kernel, netdev, pabeni,
	przemyslaw.kitszel, stable, zilin



On 11.06.2026 04:02, Dawei Feng wrote:
> Hi Marcin,
> 
> Thanks for your review.
> 
> On Tue, 9 Jun 2026 at 16:27:20 Marcin Szycik wrote:
>> IMO last two paragraphs should not be included in commit message,
>> rather after ---.
> 
> The reason the manual inspection and testing commentary was placed above
> the `---` line is that we were strictly following the example template
> provided in Documentation/process/researcher-guidelines.rst. 
> 
> In the researcher-guidelines[1], the example explicitly places the build
> and hardware testing disclaimer before the Signed-off-by tags, which is
> why we included it directly in the commit message.
> 
> Please let me know if you would like a v2 to adjust the position of the
> mentioned commit log details.

Thanks for linking the docs, now I see the commit message is exactly as
recommended.

Thanks,
Marcin
>> Correct me if I'm wrong, but looks like unroll order is reversed:
>> ice_vsi_stop_lan_tx_rings() unrolls ice_vsi_cfg_lan()
>> ice_vsi_free_rx_rings() unrolls ice_vsi_setup_rx_rings()
>> (was reversed before this patch too, but since we're fixing it, might as well)
> 
> You are right. I'll update it in v2.
> 
> [1] https://docs.kernel.org/process/researcher-guidelines.html
> 
> Best regards,
> Dawei


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

end of thread, other threads:[~2026-06-11  9:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-09 12:50 [PATCH net] ice: fix memory leak in ice_lbtest_prepare_rings() Dawei Feng
2026-06-09 14:27 ` [Intel-wired-lan] " Marcin Szycik
2026-06-09 22:27   ` Jacob Keller
2026-06-11  2:02     ` Dawei Feng
2026-06-11  9:57       ` Marcin Szycik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox