From nobody Tue May 14 18:32:05 2024 Delivered-To: importer2@patchew.org Received-SPF: pass (zohomail.com: domain of vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; envelope-from=linux-kernel-owner@vger.kernel.org; helo=vger.kernel.org; Authentication-Results: mx.zohomail.com; spf=pass (zohomail.com: domain of vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail(p=none dis=none) header.from=gmail.com ARC-Seal: i=1; a=rsa-sha256; t=1621638093; cv=none; d=zohomail.com; s=zohoarc; b=B9XkHQ5zmfym+vcIl67Sl5SRH0ubUh1y/Q/FwBMhdWl+Pk8pvofNMcuQNq0lebMIUQmoVnm2XAknBgI/FNDHQtPjf6t51z6ltkBzkoeR3+K7RL/cDOntMhnrC8QOgekyGhkNHYr8gWju9OZdUY5Np13MA30y/NayKGn0vwO5Uto= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1621638093; h=Content-Transfer-Encoding:Cc:Date:From:List-Id:MIME-Version:Message-ID:Subject:To; bh=hdEozCx62Dj0/Oxyn7rnNFecssWxN+7bYDSJ3hSUqg8=; b=Zz1eS7FPdZIZpmboVEt8R2fQCS1RYRGeHwce4zHWnXAfsu+NIA/67DMUNCmgxI1zRk/aGUTEF/ZrfQkuZJqQiIbSGEOhiLTNqNAa/CWbIXNVf2TYnvrYSlH/m3tRMpYdCqo9GH4Saj9MXnaj0xXkMyxgwtXTrgE71ejUA7VSjQM= ARC-Authentication-Results: i=1; mx.zohomail.com; spf=pass (zohomail.com: domain of vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mx.zohomail.com with SMTP id 1621638093515761.1158709856104; Fri, 21 May 2021 16:01:33 -0700 (PDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230051AbhEUXCz (ORCPT ); Fri, 21 May 2021 19:02:55 -0400 Received: from terran.cs.ucr.edu ([169.235.31.181]:54450 "EHLO terran.cs.ucr.edu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229535AbhEUXCr (ORCPT ); Fri, 21 May 2021 19:02:47 -0400 Received: from hang by terran.cs.ucr.edu with local (Exim 4.82) (envelope-from ) id 1lkDhl-0006WH-D5; Fri, 21 May 2021 15:33:17 -0700 X-Greylist: delayed 2043 seconds by postgrey-1.27 at vger.kernel.org; Fri, 21 May 2021 19:02:46 EDT From: Hang Zhang Cc: Solomon Peachy , Kalle Valo , "David S . Miller" , Jakub Kicinski , Jia-Ju Bai , Wei Yongjun , linux-wireless@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Hang Zhang Subject: [PATCH] cw1200: Revert unnecessary patches that fix unreal use-after-free bugs Date: Fri, 21 May 2021 15:32:38 -0700 Message-Id: <20210521223238.25020-1-zh.nvgt@gmail.com> X-Mailer: git-send-email 2.29.0 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" A previous commit 4f68ef64cd7f ("cw1200: Fix concurrency=20 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=20 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=20 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=20 addressed in a subsequent commit 51c8d24101c7 ("cw1200: fix missing=20 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: 4f68ef64cd7f ("cw1200: Fix concurrency use-after-free bugs in cw1200= _hw_scan()") Fixes: 51c8d24101c7 ("cw1200: fix missing unlock on error in cw1200_hw_scan= ()") Signed-off-by: Hang Zhang --- drivers/net/wireless/st/cw1200/scan.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/drivers/net/wireless/st/cw1200/scan.c b/drivers/net/wireless/s= t/cw1200/scan.c index 988581cc134b..1f856fbbc0ea 100644 --- a/drivers/net/wireless/st/cw1200/scan.c +++ b/drivers/net/wireless/st/cw1200/scan.c @@ -75,30 +75,27 @@ int cw1200_hw_scan(struct ieee80211_hw *hw, if (req->n_ssids > WSM_SCAN_MAX_NUM_OF_SSIDS) return -EINVAL; =20 - /* will be unlocked in cw1200_scan_work() */ - down(&priv->scan.lock); - mutex_lock(&priv->conf_mutex); - frame.skb =3D 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; - } =20 if (req->ie_len) skb_put_data(frame.skb, req->ie, req->ie_len); =20 + /* will be unlocked in cw1200_scan_work() */ + down(&priv->scan.lock); + mutex_lock(&priv->conf_mutex); + ret =3D wsm_set_template_frame(priv, &frame); if (!ret) { /* Host want to be the probe responder. */ ret =3D 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; } =20 @@ -120,8 +117,8 @@ int cw1200_hw_scan(struct ieee80211_hw *hw, ++priv->scan.n_ssids; } =20 - dev_kfree_skb(frame.skb); mutex_unlock(&priv->conf_mutex); + dev_kfree_skb(frame.skb); queue_work(priv->workqueue, &priv->scan.work); return 0; } --=20 2.31.1