Re: [PATCH v2] mac80211: fix NULL pointer dereference in ieee80211_key_alloc()

[Johannes Berg <johannes@sipsolutions.net>]

On Sun, 2011-03-27 at 13:31 +0200, Petr Štetiar wrote:
> The ieee80211_key struct can be kfree()d several times in the function, for
> example if some of the key setup functions fails beforehand, but there's no
> check if the struct is still valid before we call memcpy() and INIT_LIST_HEAD()
> on it.  In some cases (like it was in my case), if there's missing aes-generic
> module it could lead to the following kernel OOPS:
> 
> 	Unable to handle kernel NULL pointer dereference at virtual address 0000018c
> 	....
> 	PC is at memcpy+0x80/0x29c
> 	...
> 	Backtrace:
> 	[<bf11c5e4>] (ieee80211_key_alloc+0x0/0x234 [mac80211]) from [<bf1148b4>] (ieee80211_add_key+0x70/0x12c [mac80211])
> 	[<bf114844>] (ieee80211_add_key+0x0/0x12c [mac80211]) from [<bf070cc0>] (__cfg80211_set_encryption+0x2a8/0x464 [cfg80211])
> 
> Signed-off-by: Petr Štetiar <ynezz@true.cz>
> CC: Johannes Berg <johannes@sipsolutions.net>

Thanks!

Reviewed-by: Johannes Berg <johannes@sipsolutions.net>

> ---
>  net/mac80211/key.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mac80211/key.c b/net/mac80211/key.c
> index 8c02469..09cf1f2 100644
> --- a/net/mac80211/key.c
> +++ b/net/mac80211/key.c
> @@ -342,7 +342,7 @@ struct ieee80211_key *ieee80211_key_alloc(u32 cipher, int idx, size_t key_len,
>  		if (IS_ERR(key->u.ccmp.tfm)) {
>  			err = PTR_ERR(key->u.ccmp.tfm);
>  			kfree(key);
> -			key = ERR_PTR(err);
> +			return ERR_PTR(err);
>  		}
>  		break;
>  	case WLAN_CIPHER_SUITE_AES_CMAC:
> @@ -360,7 +360,7 @@ struct ieee80211_key *ieee80211_key_alloc(u32 cipher, int idx, size_t key_len,
>  		if (IS_ERR(key->u.aes_cmac.tfm)) {
>  			err = PTR_ERR(key->u.aes_cmac.tfm);
>  			kfree(key);
> -			key = ERR_PTR(err);
> +			return ERR_PTR(err);
>  		}
>  		break;
>  	}