From nobody Wed Jun 12 16:41:29 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4VzrsQ4ZGGz5N37w; Wed, 12 Jun 2024 16:41:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4VzrsP6Hyzz4gXC; Wed, 12 Jun 2024 16:41:29 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1718210489; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Xhwqn3i9cOMm/t3jloFgazq2HNgfPdFsHV+4IH5CyBo=; b=gjOBZcJUtmBBjpyGSdNeWirLf+wH1qOOgVUPV4LVPHkBu4ydMg/Ty2R0yJ/tUfZ1c90zQM a7LT44R/DpWjKBWYN9JwvCoe54h9RvmQuqGYMU/OX+q43tlMvuOxiJAyrEo3j+fzCOGvA/ iSxWDuBTK2vOo2YulVDL46TkU6oD4HqdxV6GBv1p+5u8d9xxvjKnqEMUwJUHHlXd4veOTv h4QeO2nwP3nG0XF1Sr+jke2oJgQXhOSIFPE/9Eklh0T5pFc2qMcVVIGDLBXmw01eOeYQMQ P/t8zO4ww4ZUS6iH4VCyw3j3EGb6DBvVJZKpCjT1XemLBk4ZzTytDMg4nIZyVQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1718210489; a=rsa-sha256; cv=none; b=kxA9DZpczXTtLK2ffYPmlBdXoJMsEYX4UGKV7/2sWvHj3trdI+Mn9wTz4b+aptgoQ4iIzS BIYnJTWk0+Qn0HbBNUyI+KjfS4FM3TpHZhPj9RPjUGPKK4kjlUxZdixbSfaVD5696qIDlh 9+tcjT1+N6Me49Im2X5I6sKDqYu7iUHQw1+SVCktpAtu6/wvt53X9k1TkOAV7Mes4tXMRL o5hLpiY48V0I1jMZthhavbweeCxU/sWkjZ9JBIwCAw+Op0Y2RVoA5uZFotxRY9bb1TTbp6 By9tx14vXz2PloNbjgfbIZhWLRlDhCsgjj1VHUwzrNQ91Fqf/3zlK5XA2c/Wsw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1718210489; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Xhwqn3i9cOMm/t3jloFgazq2HNgfPdFsHV+4IH5CyBo=; b=SW6V0H39jx4kdnMn5pH8STUGAkW9RFASeIM4T6l5giz5C9AQ7nPXlKJoHITXfU/NMTwOln S7bBWYWshJUP3MB440vLdDNSi9kjHsdJoUAxbDZtrK8HCSgafgvIauacdtQLhEjU2a4sqJ rq8yrky/TiTcDZYA6DPoUjJCb4xmMNklCxQvQCZpOYdED5t38SKE8FjKGrSTuGWyUmgvEr QXmQkIG3Of1l78AxJ3v07EGdY1976GOBfH0Lh4FQ8KotZlIqnJUgm6KcnLNM7LfFHmqc7i qa5n5hc5yIOo3Ec1JOWJotcj3oeP8Qz6XAlwwX7lNbamjhalhUtRKwcKnQJRMg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4VzrsP5wCsz114W; Wed, 12 Jun 2024 16:41:29 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 45CGfTXN046996; Wed, 12 Jun 2024 16:41:29 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 45CGfTcf046993; Wed, 12 Jun 2024 16:41:29 GMT (envelope-from git) Date: Wed, 12 Jun 2024 16:41:29 GMT Message-Id: <202406121641.45CGfTcf046993@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: "Bjoern A. Zeeb" Subject: git: 9e36362f6a87 - stable/14 - LinuxKPI: 802.11: close race lkpi_sta_scan_to_auth()/(*iv_update_bss)() List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: bz X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: 9e36362f6a871215c269edb43b87daba55b495e5 Auto-Submitted: auto-generated The branch stable/14 has been updated by bz: URL: https://cgit.FreeBSD.org/src/commit/?id=9e36362f6a871215c269edb43b87daba55b495e5 commit 9e36362f6a871215c269edb43b87daba55b495e5 Author: Bjoern A. Zeeb AuthorDate: 2024-02-18 20:57:51 +0000 Commit: Bjoern A. Zeeb CommitDate: 2024-06-12 13:59:32 +0000 LinuxKPI: 802.11: close race lkpi_sta_scan_to_auth()/(*iv_update_bss)() We have to unlock the net80211 ic lock in order to be able to call sleepable downcalls to the driver/firmware; a 2nd thread may go through net80211::join1() and (*iv_update_bss)() after we checked and unlocked. Re-check status at the end of the function under the ic lock so that we do not accidentally set lvif_bss_synched to true again despite it no longer being true. This should fix a race where we lost the (*iv_update_bss)() state during startup where one SCAN->AUTH is followed by a (then) AUTH->AUTH and lkpi_sta_a_to_a() did the wrong thing. Once we re-consider net80211 state and allowing a second join on a different node or iv_bss update without previously tearing down the older node we can likely undo a lot of these extra checks and workarounds. Sponsored by: The FreeBSD Foundation (updated version) Tested by: emaste (on and off) Reviewd by: cc Differential Revision: https://reviews.freebsd.org/D43967 (cherry picked from commit 105b9df26ee0286f3a5a7d191075e068dee1c4a2) --- sys/compat/linuxkpi/common/src/linux_80211.c | 66 +++++++++++++++++++--------- 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/sys/compat/linuxkpi/common/src/linux_80211.c b/sys/compat/linuxkpi/common/src/linux_80211.c index 6c96f1541488..728103778e4e 100644 --- a/sys/compat/linuxkpi/common/src/linux_80211.c +++ b/sys/compat/linuxkpi/common/src/linux_80211.c @@ -1276,25 +1276,6 @@ lkpi_sta_scan_to_auth(struct ieee80211vap *vap, enum ieee80211_state nstate, int lsta = ni->ni_drv_data; LKPI_80211_LVIF_LOCK(lvif); - /* Re-check given (*iv_update_bss) could have happened. */ - /* XXX-BZ KASSERT later? or deal as error? */ - if (lvif->lvif_bss_synched || lvif->lvif_bss != NULL) - ic_printf(vap->iv_ic, "%s:%d: lvif %p vap %p iv_bss %p lvif_bss %p " - "lvif_bss->ni %p synched %d, ni %p lsta %p\n", __func__, __LINE__, - lvif, vap, vap->iv_bss, lvif->lvif_bss, - (lvif->lvif_bss != NULL) ? lvif->lvif_bss->ni : NULL, - lvif->lvif_bss_synched, ni, lsta); - - /* - * Reference the ni for this cache of lsta/ni on lvif->lvif_bss - * essentially out lsta version of the iv_bss. - * Do NOT use iv_bss here anymore as that may have diverged from our - * function local ni already and would lead to inconsistencies. - */ - ieee80211_ref_node(ni); - lvif->lvif_bss = lsta; - lvif->lvif_bss_synched = true; - /* Insert the [l]sta into the list of known stations. */ TAILQ_INSERT_TAIL(&lvif->lsta_head, lsta, lsta_entry); LKPI_80211_LVIF_UNLOCK(lvif); @@ -1343,11 +1324,56 @@ lkpi_sta_scan_to_auth(struct ieee80211vap *vap, enum ieee80211_state nstate, int * (ideally we'd do that on a callback for something else ...) */ + LKPI_80211_LHW_UNLOCK(lhw); + IEEE80211_LOCK(vap->iv_ic); + + LKPI_80211_LVIF_LOCK(lvif); + /* Re-check given (*iv_update_bss) could have happened while we were unlocked. */ + if (lvif->lvif_bss_synched || lvif->lvif_bss != NULL || + lsta->ni != vap->iv_bss) + ic_printf(vap->iv_ic, "%s:%d: lvif %p vap %p iv_bss %p lvif_bss %p " + "lvif_bss->ni %p synched %d, ni %p lsta %p\n", __func__, __LINE__, + lvif, vap, vap->iv_bss, lvif->lvif_bss, + (lvif->lvif_bss != NULL) ? lvif->lvif_bss->ni : NULL, + lvif->lvif_bss_synched, ni, lsta); + + /* + * Reference the "ni" for caching the lsta/ni in lvif->lvif_bss. + * Given we cache lsta we use lsta->ni instead of ni here (even though + * lsta->ni == ni) to be distinct from the rest of the code where we do + * assume that ni == vap->iv_bss which it may or may not be. + * So do NOT use iv_bss here anymore as that may have diverged from our + * function local ni already while ic was unlocked and would lead to + * inconsistencies. Go and see if we lost a race and do not update + * lvif_bss_synched in that case. + */ + ieee80211_ref_node(lsta->ni); + lvif->lvif_bss = lsta; + if (lsta->ni == vap->iv_bss) { + lvif->lvif_bss_synched = true; + } else { + /* Set to un-synched no matter what. */ + lvif->lvif_bss_synched = false; + /* + * We do not error as someone has to take us down. + * If we are followed by a 2nd, new net80211::join1() going to + * AUTH lkpi_sta_a_to_a() will error, lkpi_sta_auth_to_{scan,init}() + * will take the lvif->lvif_bss node down eventually. + * What happens with the vap->iv_bss node will entirely be up + * to net80211 as we never used the node beyond alloc()/free() + * and we do not hold an extra reference for that anymore given + * ni : lsta == 1:1. + */ + } + LKPI_80211_LVIF_UNLOCK(lvif); + goto out_relocked; + out: LKPI_80211_LHW_UNLOCK(lhw); IEEE80211_LOCK(vap->iv_ic); +out_relocked: /* - * Release the reference that keop the ni stable locally + * Release the reference that kept the ni stable locally * during the work of this function. */ if (ni != NULL)