Skip to content

Commit

Permalink
cw1200: Revert unnecessary patches that fix unreal use-after-free bugs
Browse files Browse the repository at this point in the history
[ Upstream commit 3f60f46 ]

A previous commit 4f68ef6 ("cw1200: Fix concurrency
use-after-free bugs in cw1200_hw_scan()") tried to fix a seemingly
use-after-free bug between cw1200_bss_info_changed() and
cw1200_hw_scan(), where the former frees a sk_buff pointed
to by frame.skb, and the latter accesses the sk_buff
pointed to by frame.skb. However, this issue should be a
false alarm because:

(1) "frame.skb" is not a shared variable between the above
two functions, because "frame" is a local function variable,
each of the two functions has its own local "frame" - they
just happen to have the same variable name.

(2) the sk_buff(s) pointed to by these two "frame.skb" are
also two different object instances, they are individually
allocated by different dev_alloc_skb() within the two above
functions. To free one object instance will not invalidate
the access of another different one.

Based on these facts, the previous commit should be unnecessary.
Moreover, it also introduced a missing unlock which was
addressed in a subsequent commit 51c8d24 ("cw1200: fix missing
unlock on error in cw1200_hw_scan()"). Now that the
original use-after-free is unreal, these two commits should
be reverted. This patch performs the reversion.

Fixes: 4f68ef6 ("cw1200: Fix concurrency use-after-free bugs in cw1200_hw_scan()")
Fixes: 51c8d24 ("cw1200: fix missing unlock on error in cw1200_hw_scan()")
Signed-off-by: Hang Zhang <zh.nvgt@gmail.com>
Acked-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Link: https://lore.kernel.org/r/20210521223238.25020-1-zh.nvgt@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
bugbot97 authored and gregkh committed Jul 14, 2021
1 parent 164ede1 commit 9569929
Showing 1 changed file with 7 additions and 10 deletions.
17 changes: 7 additions & 10 deletions drivers/net/wireless/st/cw1200/scan.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,30 +75,27 @@ int cw1200_hw_scan(struct ieee80211_hw *hw,
if (req->n_ssids > WSM_SCAN_MAX_NUM_OF_SSIDS)
return -EINVAL;

/* will be unlocked in cw1200_scan_work() */
down(&priv->scan.lock);
mutex_lock(&priv->conf_mutex);

frame.skb = ieee80211_probereq_get(hw, priv->vif->addr, NULL, 0,
req->ie_len);
if (!frame.skb) {
mutex_unlock(&priv->conf_mutex);
up(&priv->scan.lock);
if (!frame.skb)
return -ENOMEM;
}

if (req->ie_len)
skb_put_data(frame.skb, req->ie, req->ie_len);

/* will be unlocked in cw1200_scan_work() */
down(&priv->scan.lock);
mutex_lock(&priv->conf_mutex);

ret = wsm_set_template_frame(priv, &frame);
if (!ret) {
/* Host want to be the probe responder. */
ret = wsm_set_probe_responder(priv, true);
}
if (ret) {
dev_kfree_skb(frame.skb);
mutex_unlock(&priv->conf_mutex);
up(&priv->scan.lock);
dev_kfree_skb(frame.skb);
return ret;
}

Expand All @@ -120,8 +117,8 @@ int cw1200_hw_scan(struct ieee80211_hw *hw,
++priv->scan.n_ssids;
}

dev_kfree_skb(frame.skb);
mutex_unlock(&priv->conf_mutex);
dev_kfree_skb(frame.skb);
queue_work(priv->workqueue, &priv->scan.work);
return 0;
}
Expand Down

0 comments on commit 9569929

Please sign in to comment.