Re: [Intel-wired-lan] [PATCH net] ice: fix memory leak in ice_lbtest_prepare_rings()

[Jacob Keller <jacob.e.keller@intel.com>]

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