From patchwork Tue Sep 19 06:34:15 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johannes Berg X-Patchwork-Id: 13390860 X-Patchwork-Delegate: johannes@sipsolutions.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C61E4CD3440 for ; Tue, 19 Sep 2023 06:34:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231519AbjISGed (ORCPT ); Tue, 19 Sep 2023 02:34:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50244 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229483AbjISGeb (ORCPT ); Tue, 19 Sep 2023 02:34:31 -0400 Received: from sipsolutions.net (s3.sipsolutions.net [IPv6:2a01:4f8:242:246e::2]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 98E8F115 for ; Mon, 18 Sep 2023 23:34:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sipsolutions.net; s=mail; h=Content-Transfer-Encoding:MIME-Version: Message-ID:Date:Subject:Cc:To:From:Content-Type:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-To:Resent-Cc: Resent-Message-ID:In-Reply-To:References; bh=LRSu3FBEVbqu2CH/FpI6JhHJOW8DAQtGfL9v8FXZNts=; t=1695105264; x=1696314864; b=U5gT7Oy469/mllFZQHd2OtYfin1OV1JFow2ggQVBV2fWeUEuvBzUpZUhLvy7hB9IFLR0mU7Y1w+ OxH3grgMxIq3Qt7VFTFQo4p3gj0lyUscD3Jk8b6Vtv9/09qbLRG5JziFe0AC7/SlNz3rSSV8LEdaQ d8v8UBUEmTnRCHxKsw0d+3/IZYpaegd0t9pqfOYEmXKoKCvHzW1PUlnEYx+dtqUMmrny4NEGUoRyG 7XZsLU6AUZD32Zc0HIWuDZyOsWvVKCFBNGzzTsPdShqh7vDfltStWp6MwhKGzXvVq4p7vSf4tzdgN blZtUQlU5mbYTRb7hGO2Ie07+FejYDNIqy6Q==; Received: by sipsolutions.net with esmtpsa (TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.96) (envelope-from ) id 1qiUJQ-008tsE-25; Tue, 19 Sep 2023 08:34:21 +0200 From: Johannes Berg To: linux-wireless@vger.kernel.org Cc: Johannes Berg , Dan Carpenter Subject: [PATCH 1/2] wifi: mac80211: fix potential key use-after-free Date: Tue, 19 Sep 2023 08:34:15 +0200 Message-ID: <20230919083414.d97c3dfda70e.I058ce0718e73c97de9cd19f499d40891df984ece@changeid> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org From: Johannes Berg When ieee80211_key_link() is called by ieee80211_gtk_rekey_add() but returns 0 due to KRACK protection (identical key reinstall), ieee80211_gtk_rekey_add() will still return a pointer into the key, in a potential use-after-free. This normally doesn't happen since it's only called by iwlwifi in case of WoWLAN rekey offload which has its own KRACK protection, but still better to fix, do that by returning an error code and converting that to success on the cfg80211 boundary only, leaving the error for bad callers of ieee80211_gtk_rekey_add(). Reported-by: Dan Carpenter Fixes: fdf7cb4185b6 ("mac80211: accept key reinstall without changing anything") Signed-off-by: Johannes Berg --- net/mac80211/cfg.c | 3 +++ net/mac80211/key.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index 45e7a5d9c7d9..e883c41a2163 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -566,6 +566,9 @@ static int ieee80211_add_key(struct wiphy *wiphy, struct net_device *dev, } err = ieee80211_key_link(key, link, sta); + /* KRACK protection, shouldn't happen but just silently accept key */ + if (err == -EALREADY) + err = 0; out_unlock: mutex_unlock(&local->sta_mtx); diff --git a/net/mac80211/key.c b/net/mac80211/key.c index 13050dc9321f..84ba20c3e3dc 100644 --- a/net/mac80211/key.c +++ b/net/mac80211/key.c @@ -905,7 +905,7 @@ int ieee80211_key_link(struct ieee80211_key *key, */ if (ieee80211_key_identical(sdata, old_key, key)) { ieee80211_key_free_unused(key); - ret = 0; + ret = -EALREADY; goto out; } From patchwork Tue Sep 19 06:34:16 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johannes Berg X-Patchwork-Id: 13390861 X-Patchwork-Delegate: johannes@sipsolutions.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AEE70CD3441 for ; Tue, 19 Sep 2023 06:34:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231546AbjISGef (ORCPT ); Tue, 19 Sep 2023 02:34:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50256 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231250AbjISGeb (ORCPT ); Tue, 19 Sep 2023 02:34:31 -0400 Received: from sipsolutions.net (s3.sipsolutions.net [IPv6:2a01:4f8:242:246e::2]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CAB0E116 for ; Mon, 18 Sep 2023 23:34:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sipsolutions.net; s=mail; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Content-Type:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-To: Resent-Cc:Resent-Message-ID; bh=hLj4M+1nL81iF8FaWOr3YPrNl0P+2RxGbGVGGG8Ljsk=; t=1695105264; x=1696314864; b=WlCK3UykndynPb+0nU0/GusHEg/CAYpX1oF1VqIClZkIkaG rC616TSGykKTr0bd5wKmGFtX4DLJGsRCVg8P6O6GJFDQ9xeuDbWcYDOwQOkHxTQIOSIlNWe+HpuAx mahtf2GJ8MFwcPVxsn4vTVPx/vkaWQjGYaILw6X06bGpO4qFPVFPMl4u6yGGJUe0o1KGHe/O7T4w6 v21eWWbCle9incdLGUTOaL8jbFmokBz5VI0lWj8ytKWI/hiuWu7kWfyD8aPPcu9cFnx5JKO7oDdsx coc/2yuiYeYbAKP4pbfJPw7WZizpOg+NCugjvr1qBTC97ODxGV7Qup956zOrfzQA==; Received: by sipsolutions.net with esmtpsa (TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.96) (envelope-from ) id 1qiUJR-008tsE-2a; Tue, 19 Sep 2023 08:34:22 +0200 From: Johannes Berg To: linux-wireless@vger.kernel.org Cc: Johannes Berg Subject: [PATCH 2/2] wifi: mac80211: fix potential key leak Date: Tue, 19 Sep 2023 08:34:16 +0200 Message-ID: <20230919083414.a25780c91c3a.Ib812a4b56c78684bf33524ae0903fcb1e995b8bb@changeid> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230919083414.d97c3dfda70e.I058ce0718e73c97de9cd19f499d40891df984ece@changeid> References: <20230919083414.d97c3dfda70e.I058ce0718e73c97de9cd19f499d40891df984ece@changeid> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org From: Johannes Berg When returning from ieee80211_key_link(), the key needs to have been freed or successfully installed. This was missed in a number of error paths, fix it. Signed-off-by: Johannes Berg --- net/mac80211/key.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/net/mac80211/key.c b/net/mac80211/key.c index 84ba20c3e3dc..0665ff5e456e 100644 --- a/net/mac80211/key.c +++ b/net/mac80211/key.c @@ -802,6 +802,9 @@ static void ieee80211_key_destroy(struct ieee80211_key *key, void ieee80211_key_free_unused(struct ieee80211_key *key) { + if (!key) + return; + WARN_ON(key->sdata || key->local); ieee80211_key_free_common(key); } @@ -854,7 +857,7 @@ int ieee80211_key_link(struct ieee80211_key *key, * can cause warnings to appear. */ bool delay_tailroom = sdata->vif.type == NL80211_IFTYPE_STATION; - int ret = -EOPNOTSUPP; + int ret; mutex_lock(&sdata->local->key_mtx); @@ -868,8 +871,10 @@ int ieee80211_key_link(struct ieee80211_key *key, * the same cipher. Enforce the assumption for pairwise keys. */ if ((alt_key && alt_key->conf.cipher != key->conf.cipher) || - (old_key && old_key->conf.cipher != key->conf.cipher)) + (old_key && old_key->conf.cipher != key->conf.cipher)) { + ret = -EOPNOTSUPP; goto out; + } } else if (sta) { struct link_sta_info *link_sta = &sta->deflink; int link_id = key->conf.link_id; @@ -895,8 +900,10 @@ int ieee80211_key_link(struct ieee80211_key *key, /* Non-pairwise keys must also not switch the cipher on rekey */ if (!pairwise) { - if (old_key && old_key->conf.cipher != key->conf.cipher) + if (old_key && old_key->conf.cipher != key->conf.cipher) { + ret = -EOPNOTSUPP; goto out; + } } /* @@ -904,9 +911,8 @@ int ieee80211_key_link(struct ieee80211_key *key, * new version of the key to avoid nonce reuse or replay issues. */ if (ieee80211_key_identical(sdata, old_key, key)) { - ieee80211_key_free_unused(key); ret = -EALREADY; - goto out; + goto unlock; } key->local = sdata->local; @@ -930,7 +936,11 @@ int ieee80211_key_link(struct ieee80211_key *key, ieee80211_key_free(key, delay_tailroom); } + key = NULL; + out: + ieee80211_key_free_unused(key); + unlock: mutex_unlock(&sdata->local->key_mtx); return ret;