Key information in wext.connect is not reset on (re)connect and can hold
data from a previous connection.
Reset key data to avoid that drivers or mac80211 incorrectly detect a
WEP connection request and access the freed or already reused memory.
Additionally optimize cfg80211_sme_connect() and avoid an useless
schedule of conn_work.
Fixes: fffd0934b939 ("cfg80211: rework key operation")
Cc: [email protected]
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Alexander Wetzel <[email protected]>
---
I was first wondering if the dangling scheduled work was part of the
problem. It's kind of pointless to schedule a work and then just do the job
yourself. While it turned out to be benign I still added it to the fix here.
Alexander
---
net/wireless/sme.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 123248b2c0be..8d8176e31e31 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -285,6 +285,15 @@ void cfg80211_conn_work(struct work_struct *work)
wiphy_unlock(&rdev->wiphy);
}
+static void cfg80211_step_auth_next(struct cfg80211_conn *conn,
+ struct cfg80211_bss *bss)
+{
+ memcpy(conn->bssid, bss->bssid, ETH_ALEN);
+ conn->params.bssid = conn->bssid;
+ conn->params.channel = bss->channel;
+ conn->state = CFG80211_CONN_AUTHENTICATE_NEXT;
+}
+
/* Returned bss is reference counted and must be cleaned up appropriately. */
static struct cfg80211_bss *cfg80211_get_conn_bss(struct wireless_dev *wdev)
{
@@ -302,10 +311,7 @@ static struct cfg80211_bss *cfg80211_get_conn_bss(struct wireless_dev *wdev)
if (!bss)
return NULL;
- memcpy(wdev->conn->bssid, bss->bssid, ETH_ALEN);
- wdev->conn->params.bssid = wdev->conn->bssid;
- wdev->conn->params.channel = bss->channel;
- wdev->conn->state = CFG80211_CONN_AUTHENTICATE_NEXT;
+ cfg80211_step_auth_next(wdev->conn, bss);
schedule_work(&rdev->conn_work);
return bss;
@@ -597,7 +603,12 @@ static int cfg80211_sme_connect(struct wireless_dev *wdev,
wdev->conn->params.ssid_len = wdev->u.client.ssid_len;
/* see if we have the bss already */
- bss = cfg80211_get_conn_bss(wdev);
+ bss = cfg80211_get_bss(wdev->wiphy, wdev->conn->params.channel,
+ wdev->conn->params.bssid,
+ wdev->conn->params.ssid,
+ wdev->conn->params.ssid_len,
+ wdev->conn_bss_type,
+ IEEE80211_PRIVACY(wdev->conn->params.privacy));
if (prev_bssid) {
memcpy(wdev->conn->prev_bssid, prev_bssid, ETH_ALEN);
@@ -608,6 +619,7 @@ static int cfg80211_sme_connect(struct wireless_dev *wdev,
if (bss) {
enum nl80211_timeout_reason treason;
+ cfg80211_step_auth_next(wdev->conn, bss);
err = cfg80211_conn_do_work(wdev, &treason);
cfg80211_put_bss(wdev->wiphy, bss);
} else {
@@ -1464,6 +1476,13 @@ int cfg80211_connect(struct cfg80211_registered_device *rdev,
} else {
if (WARN_ON(connkeys))
return -EINVAL;
+
+ /* connect can point to wdev->connect
+ * and may hold outdated key data
+ */
+ connect->key = NULL;
+ connect->key_len = 0;
+ connect->key_idx = 0;
}
wdev->connect_keys = connkeys;
--
2.39.0
On 24.01.23 12:45, Alexander Wetzel wrote:
> Key information in wext.connect is not reset on (re)connect and can hold
> data from a previous connection.
>
> Reset key data to avoid that drivers or mac80211 incorrectly detect a
> WEP connection request and access the freed or already reused memory.
>
> Additionally optimize cfg80211_sme_connect() and avoid an useless
> schedule of conn_work.
>
> Fixes: fffd0934b939 ("cfg80211: rework key operation")
> Cc: [email protected]
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Alexander Wetzel <[email protected]>
Send out an outdated version. Still does the job but I'll send a v2.
Which will reset more key data and updates the comment.
Alexander