From nobody Thu Sep 12 08:37:10 2024 X-Original-To: dev-commits-src-main@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 4X49m648Mkz5VKTw; Thu, 12 Sep 2024 08:37:10 +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 "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4X49m63Fgdz4dNl; Thu, 12 Sep 2024 08:37:10 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1726130230; 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=g+a0LQ0s6K7FEjmDQ9BH3iZ/FcsW0IY5VhLp165zjjk=; b=gkmAipelvmrjVfXi/G6ej7CL1R6DiJb7G/bFUeWgJz2LvwMGb024PsdX80p0XUJzd+dyoi D6TODsVncg34FxeBvBivB4SPCS/6xz7Wwm0zXITqWHz/K8cWPcju6celcqBrSFFs8gH4IB 1ki2efvNW4SVmGCIx7C2XgLJE8EMIipzitXrK/7XzXnVk8AXuwvyA01CPOkT/H1MCuyi4F QSz1snHnrdOhXfPxQPvWy1veSzFs0nRfFJmvkThJATlY1x2gdbeUjFcT5ekKdDDw3nWyz+ yDm7Uy9pJ1pPto5fgTnLKi9qtV+OiI62+96m631v4Xmc5/YuI5UHvHHOiHho/A== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1726130230; a=rsa-sha256; cv=none; b=OHAhhgPZFixxX32yh+pyEU34Py7FO57Reg58pCCqF/6S3kHn8g1ex0dhPuGDRl2HTCl2x0 Icw+5ZcmQGC0KQQxVORpA/aZ5dch08DC5QUtMuW2X5hXH+aIppaSPFnyUuG+p/s9V5QTVB zGa9X2s+EEHWeGEsBOnodXztXxNe5SLfoWf18jXnXJ/JELRgrRaoeAB8mFLaj50ORNqDdB fFpCZZcyzbUw/Gk5FR285zXZvFp9sd1e9JWf0O9I4CdIfOlh2PiXjqMChudg2S+noFqNFj O8NbJgohTivq8TNrPFkxEDj7oGgvpk+JeVCUzPZZ4XQ3qOm4JaudfWfWzRZ+5Q== 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=1726130230; 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=g+a0LQ0s6K7FEjmDQ9BH3iZ/FcsW0IY5VhLp165zjjk=; b=AIRrfd2CAdszFWFvBU3aWnGolp327zc7D6KbO1kNL7Mg1Yxd73GhIIHF1OFV2WqLuhmB6A kJBaBS+QWv0rOSV2/UaUSJsef0q/XSJHrtiFb5YL7gMLY+b7JAc/byXsK+5uWJoTM1UZEq dKGjco/q/1gtxtG/c3RUI6v4tJYvKiWnB+AJeCziLx+pnigt3s8Tr68TkIeDn3F6tgXm2b pNkLqazmXelDojhw5Vjg6tWhQo0JC0fY3lCtcV4sUTsKL78xS9/B/2LUFw03qb1/fHdF3Y NeSmfO6AP+PeTBj7nivuNocMZqKBdAK/ICruhrYFtrKCH/nt9DzEBQ+7Pquq5w== 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 4X49m62nLQz17JH; Thu, 12 Sep 2024 08:37:10 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 48C8bAKY040916; Thu, 12 Sep 2024 08:37:10 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 48C8bAKC040913; Thu, 12 Sep 2024 08:37:10 GMT (envelope-from git) Date: Thu, 12 Sep 2024 08:37:10 GMT Message-Id: <202409120837.48C8bAKC040913@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: 3acf7e0da487 - main - if_ovpn: avoid LOR between ovpn and UDP locks List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 3acf7e0da487c08de39d04ac069ae73fb8a488c0 Auto-Submitted: auto-generated The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=3acf7e0da487c08de39d04ac069ae73fb8a488c0 commit 3acf7e0da487c08de39d04ac069ae73fb8a488c0 Author: Kristof Provost AuthorDate: 2024-09-09 12:14:03 +0000 Commit: Kristof Provost CommitDate: 2024-09-12 07:44:19 +0000 if_ovpn: avoid LOR between ovpn and UDP locks When we install the tunneling function we had the ovpn lock, and then took the UDP lock. During normal data flow we are called with the UDP lock held and then take the ovpn lock. This naturally produces a lock order reversal warning. Avoid this by releasing the ovpn lock before installing the tunnel function. This is safe, in that installing the tunnel function does not fail (other than with EBUSY, which would mean another thread has already installed the function). On cleanup the problem is more difficult, in that we cannot reasonably release the ovpn lock before we can remove the tunneling function callback. Solve this by delaying the removal of the tunnel callback until the ovpn_softc is cleaned up. It's still safe for ovpn_udp_input() to be caled when all peers are removed. That will only increment counters (which are still allocated), discover there are no peers and then pass the message on to userspace, if any userspace users of the socket remain. We ensure that the socket object remains valid by holding a reference, which we release when we remove the ovpn_softc. This removes the need for per-peer reference counting on the socket, so remove that. Reviewed by: zlei Sponsored by: Rubicon Communications, LLC ("Netgate") Differential Revision:: https://reviews.freebsd.org/D46616 --- sys/net/if_ovpn.c | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/sys/net/if_ovpn.c b/sys/net/if_ovpn.c index 349fa5b7cd13..e2c1dc7f7fc5 100644 --- a/sys/net/if_ovpn.c +++ b/sys/net/if_ovpn.c @@ -390,15 +390,11 @@ ovpn_rele_so(struct ovpn_softc *sc) has_peers = ovpn_has_peers(sc); - /* Only remove the tunnel function if we're releasing the socket for - * the last peer. */ - if (! has_peers) - (void)udp_set_kernel_tunneling(sc->so, NULL, NULL, NULL); - - sorele(sc->so); - - if (! has_peers) - sc->so = NULL; + if (! has_peers) { + MPASS(sc->peercount == 0); + } else { + MPASS(sc->peercount > 0); + } } static void @@ -628,26 +624,23 @@ ovpn_new_peer(struct ifnet *ifp, const nvlist_t *nvl) if (sc->so != NULL && so != sc->so) goto error_locked; - if (sc->so == NULL) + if (sc->so == NULL) { sc->so = so; + /* + * Maintain one extra ref so the socket doesn't go away until + * we're destroying the ifp. + */ + soref(sc->so); + } /* Insert the peer into the list. */ RB_INSERT(ovpn_kpeers, &sc->peers, peer); sc->peercount++; - soref(sc->so); - - ret = udp_set_kernel_tunneling(sc->so, ovpn_udp_input, NULL, sc); - if (ret == EBUSY) { - /* Fine, another peer already set the input function. */ - ret = 0; - } - if (ret != 0) { - RB_REMOVE(ovpn_kpeers, &sc->peers, peer); - sc->peercount--; - goto error_locked; - } OVPN_WUNLOCK(sc); + ret = udp_set_kernel_tunneling(sc->so, ovpn_udp_input, NULL, sc); + MPASS(ret == 0 || ret == EBUSY); + ret = 0; goto done; @@ -2493,12 +2486,21 @@ static void ovpn_clone_destroy_cb(struct epoch_context *ctx) { struct ovpn_softc *sc; + int ret __diagused; sc = __containerof(ctx, struct ovpn_softc, epoch_ctx); MPASS(sc->peercount == 0); MPASS(RB_EMPTY(&sc->peers)); + if (sc->so != NULL) { + CURVNET_SET(sc->ifp->if_vnet); + ret = udp_set_kernel_tunneling(sc->so, NULL, NULL, NULL); + MPASS(ret == 0); + sorele(sc->so); + CURVNET_RESTORE(); + } + COUNTER_ARRAY_FREE(sc->counters, OVPN_COUNTER_SIZE); if_free(sc->ifp);