linux-kernel - Re: [PATCH net v2] net: wireless: Use kfree

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-next>] [day] [month] [year] [list]

Message-ID: <2023071950-nervous-grub-5ee3@gregkh>
Date:   Wed, 19 Jul 2023 07:36:04 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     王明-软件底层技术部 
        <machel@...o.com>
Cc:     Ajay Singh <ajay.kathat@...rochip.com>,
        Claudiu Beznea <claudiu.beznea@...rochip.com>,
        Kalle Valo <kvalo@...nel.org>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "opensource.kernel" <opensource.kernel@...o.com>
Subject: Re: [PATCH net v2] net: wireless: Use kfree_sensitive instead of
 kfree

On Wed, Jul 19, 2023 at 02:21:16AM +0000, 王明-软件底层技术部 wrote:
> key contains private part of the key, so better use
> kfree_sensitive to free it.
> 
> Fixes: 7cec84fdfd88 ("staging: wilc1000: split add_key() to avoid line over 80 chars")
> Signed-off-by: Wang Ming <machel@...o.com>

Why not also use 王明-软件底层技术部 <machel@...o.com> here as well?


> ---
>  drivers/net/wireless/microchip/wilc1000/cfg80211.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

No change log from what changed from version 1?



> 
> diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
> index b545d93c6e37..45bcadeba2da 100644
> --- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
> +++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
> @@ -518,7 +518,7 @@ static int wilc_wfi_cfg_allocate_wpa_igtk_entry(struct wilc_priv *priv, u8 idx)
>  static int wilc_wfi_cfg_copy_wpa_info(struct wilc_wfi_key *key_info,
>  				      struct key_params *params)
>  {
> -	kfree(key_info->key);
> +	kfree_sensitive(key_info->key);
>  
>  	key_info->key = kmemdup(params->key, params->key_len, GFP_KERNEL);
>  	if (!key_info->key)
> @@ -656,7 +656,7 @@ static int del_key(struct wiphy *wiphy, struct net_device *netdev, int link_id,
>  	if (!pairwise && (key_index == 4 || key_index == 5)) {
>  		key_index -= 4;
>  		if (priv->wilc_igtk[key_index]) {
> -			kfree(priv->wilc_igtk[key_index]->key);
> +			kfree_sensitive(priv->wilc_igtk[key_index]->key);

Normally "kfree_sensitive()" is used at the end of a function for when
kfree() of a local variable might not be called because the compiler
thinks it is smarter than us and optimizes it away.

Putting it here, in the normal operation, really doesn't do anything,
right?  There's always going to be odd data in the heap and normal
distros/users who care about that, always wipe the heap when doing new
allocations as that's a kernel config option.

So what exactly is this "fixing" here?  What is the bug?

thanks,

greg k-h