netdev - Re: [PATCH iwl-net v1] ice: fix bug with suspend and rebuild

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cb61bb19-ac6a-4271-b699-69b94d55b945@intel.com>
Date: Tue, 5 Mar 2024 09:35:51 -0800
From: Jesse Brandeburg <jesse.brandeburg@...el.com>
To: Simon Horman <horms@...nel.org>
CC: <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>, "Robert
 Elliott" <elliott@....com>, Tony Nguyen <anthony.l.nguyen@...el.com>, "David
 S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, "Jakub
 Kicinski" <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Przemek Kitszel
	<przemyslaw.kitszel@...el.com>, Michal Swiatkowski
	<michal.swiatkowski@...ux.intel.com>
Subject: Re: [PATCH iwl-net v1] ice: fix bug with suspend and rebuild

On 3/5/2024 5:17 AM, Simon Horman wrote:
 --- a/drivers/net/ethernet/intel/ice/ice_lib.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
>> @@ -3238,7 +3238,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags)
>>  {
>>  	struct ice_vsi_cfg_params params = {};
>>  	struct ice_coalesce_stored *coalesce;
>> -	int prev_num_q_vectors = 0;
>> +	int prev_num_q_vectors;
>>  	struct ice_pf *pf;
>>  	int ret;
>>  
>> @@ -3252,13 +3252,6 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags)
>>  	if (WARN_ON(vsi->type == ICE_VSI_VF && !vsi->vf))
>>  		return -EINVAL;
>>  
>> -	coalesce = kcalloc(vsi->num_q_vectors,
>> -			   sizeof(struct ice_coalesce_stored), GFP_KERNEL);
>> -	if (!coalesce)
>> -		return -ENOMEM;
>> -
>> -	prev_num_q_vectors = ice_vsi_rebuild_get_coalesce(vsi, coalesce);
>> -
>>  	ret = ice_vsi_realloc_stat_arrays(vsi);
>>  	if (ret)
>>  		goto err_vsi_cfg;
>> @@ -3268,6 +3261,13 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags)
>>  	if (ret)
>>  		goto err_vsi_cfg;
> 
> Hi Jesse,
> 
> the label above will result in a call to kfree(coalesce).
> However, coalesce is now uninitialised until the following line executes.

Thanks Simon, you're right! And it figures, The internal static analysis
runs caught this only after I sent the patch to the list.

One fix is to NULL the initialization of coalesce, which solves the
problem with a one-liner, but I'll look more today at whether I should
just move the label or something else.