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: <MW5PR11MB5810FA23CCCA73A987CB8D44A3D02@MW5PR11MB5810.namprd11.prod.outlook.com>
Date: Wed, 12 Mar 2025 19:27:43 +0000
From: "Korenblit, Miriam Rachel" <miriam.rachel.korenblit@...el.com>
To: Jeff Johnson <jeff.johnson@....qualcomm.com>, Dan Carpenter
	<dan.carpenter@...aro.org>
CC: "Berg, Johannes" <johannes.berg@...el.com>, "Anjaneyulu, Pagadala Yesu"
	<pagadala.yesu.anjaneyulu@...el.com>, "Grumbach, Emmanuel"
	<emmanuel.grumbach@...el.com>, "Stern, Avraham" <avraham.stern@...el.com>,
	"Ben Shimol, Yedidya" <yedidya.ben.shimol@...el.com>, "Gabay, Daniel"
	<daniel.gabay@...el.com>, "linux-wireless@...r.kernel.org"
	<linux-wireless@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "kernel-janitors@...r.kernel.org"
	<kernel-janitors@...r.kernel.org>
Subject: RE: [PATCH next] wifi: iwlwifi: Fix uninitialized variable with
 __free()



> -----Original Message-----
> From: Jeff Johnson <jeff.johnson@....qualcomm.com>
> Sent: Wednesday, 12 March 2025 17:15
> To: Dan Carpenter <dan.carpenter@...aro.org>; Korenblit, Miriam Rachel
> <miriam.rachel.korenblit@...el.com>
> Cc: Berg, Johannes <johannes.berg@...el.com>; Anjaneyulu, Pagadala Yesu
> <pagadala.yesu.anjaneyulu@...el.com>; Grumbach, Emmanuel
> <emmanuel.grumbach@...el.com>; Stern, Avraham
> <avraham.stern@...el.com>; Ben Shimol, Yedidya
> <yedidya.ben.shimol@...el.com>; Gabay, Daniel <daniel.gabay@...el.com>;
> linux-wireless@...r.kernel.org; linux-kernel@...r.kernel.org; kernel-
> janitors@...r.kernel.org
> Subject: Re: [PATCH next] wifi: iwlwifi: Fix uninitialized variable with __free()
> 
> On 3/12/2025 1:31 AM, Dan Carpenter wrote:
> > Pointers declared with the __free(kfree) attribute need to be
> > initialized because they will be passed to kfree() on every return
> > path.  There are two return statement before the "cmd" pointer is
> > initialized so this leads to an uninitialized variable bug.
> >
> > Fixes: d1e879ec600f ("wifi: iwlwifi: add iwlmld sub-driver")
> > Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>
> > ---
> >  drivers/net/wireless/intel/iwlwifi/mld/debugfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/intel/iwlwifi/mld/debugfs.c
> > b/drivers/net/wireless/intel/iwlwifi/mld/debugfs.c
> > index c759c5c68dc0..1d4b2ad5d388 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/mld/debugfs.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/mld/debugfs.c
> > @@ -556,8 +556,8 @@ iwl_dbgfs_vif_twt_setup_write(struct iwl_mld *mld,
> char *buf, size_t count,
> >  	};
> >  	struct ieee80211_vif *vif = data;
> >  	struct iwl_mld_vif *mld_vif = iwl_mld_vif_from_mac80211(vif);
> > +	struct iwl_dhc_cmd *cmd __free(kfree) = NULL;
> 
> hmm, I thought the recommended convention was to define __free() pointers at
> the point of allocation. cleanup.h explicitly says:
> 
>  * Given that the "__free(...) = NULL" pattern for variables defined at
>  * the top of the function poses this potential interdependency problem
>  * the recommendation is to always define and assign variables in one
>  * statement and not group variable definitions at the top of the
>  * function when __free() is used.

This is used if you have multiple kfree variables (and the order of freeing matters)
or a guard() in that same function.
This is not the case here (and no reason that it will be in the future since this entire function is running under the wiphy lock),
and it is just a recommendation?
> 
> >  	struct iwl_dhc_twt_operation *dhc_twt_cmd;
> > -	struct iwl_dhc_cmd *cmd __free(kfree);
> >  	u64 target_wake_time;
> >  	u32 twt_operation, interval_exp, interval_mantissa, min_wake_duration;
> >  	u8 trigger, flow_type, flow_id, protection, tenth_param;

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ