linux-kernel - Re: [PATCH net v1] 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-prev] [day] [month] [year] [list]

Date:   Mon, 17 Jul 2023 16:35:45 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Wang Ming <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-kernel@...r.kernel.org, opensource.kernel@...o.com
Subject: Re: [PATCH net v1] net: wireless: Use kfree_sensitive instead of
 kfree

On Mon, Jul 17, 2023 at 05:34:43PM +0800, Wang Ming wrote:
> key might contain private part of the key, so better use
> kfree_sensitive to free it.

"might"?  What determines if it does, or does not, contain the private
part of the key?

Shouldn't this always be known?

> Fixes: 7cec84fdfd88 ("staging: wilc1000: split add_key() to avoid line over 80 chars")
> Signed-off-by: Wang Ming <machel@...o.com>
> ---
>  drivers/net/wireless/microchip/wilc1000/cfg80211.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> 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);

For most systems, this would have been wiped when kfree() was called due
to allocation being zeroed out, right?

But, if you want to be safe, and you know this was a private key, that's
fine to do as well, but please figure out if this really is, or is not,
a private key.

thanks,

greg k-h