From nobody Tue May 31 20:50:11 2022 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 23D081B4CB84; Tue, 31 May 2022 20:50:12 +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 4LCPYJ0SnPz3lX6; Tue, 31 May 2022 20:50:12 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1654030212; 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=fjJgf1S6x6JIz92njTf1CO08Ok1xdl3nfUMJ8MRbTAw=; b=T5gJULYUg4aQpQQEMaBG5cDjrbQ+6ANKeCFLVJgPYlsGtYMTYE6DYl4mU2pOWxCNgAEDay e6tbjxKD57VaV+Xa/G8bK77bHcRahYgB0USgzksmRi8StxkQj1tWtd0nchmPCtWAzrCj20 zAqTgYlolurqpoR74M8Mad4SV9wjlINQrxTmTKAQ/NYGS3ig/VInihFmyMoHMBmczC/y+5 kUVfPQ80Q1rgHAxKydn7R9XclPRi0jitEXlPz0aKj0HGOKgCQNy+xBZzoGr3Rwm+zA3lEQ yXtYDyftu7jftg/80G/eZwdf+NdukZhfoZq9GlmyNJxrmQyPUDtro7OApGVV+A== 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 E4B5513130; Tue, 31 May 2022 20:50:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 24VKoBuX010624; Tue, 31 May 2022 20:50:11 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 24VKoBRk010617; Tue, 31 May 2022 20:50:11 GMT (envelope-from git) Date: Tue, 31 May 2022 20:50:11 GMT Message-Id: <202205312050.24VKoBRk010617@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mateusz Guzik Subject: git: 6c92016aa685 - main - pf: fix a race against kif destruction in pf_test{,6} 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: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mjg X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 6c92016aa685486f1445f632ac3f1af1385186af Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1654030212; 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=fjJgf1S6x6JIz92njTf1CO08Ok1xdl3nfUMJ8MRbTAw=; b=KadQVm9lFa/gBBQmw9Io5dXitBSMae2Pi680Pe2gOHw1NHeB4gO8Tt4tLAUAZvccvlP4Ju Janpi1/GfUsqydjuTcrz8kc0IIQVino2piNIIDynKIohMKDyg+qDgFJSfRfFWM47xU/hwX FBrfORPovf8xNxuTWzUaBjr8PC92tx88tGMDxipJF7AXYONuZfMCqvAa8eUUiyDBVkccHw KDRFLjddhsxFaZFMJUaWXBsP321Q1nnyodDAt3oh7IJIdJwI2g+lHAeFQVTLi6RQvRPfwV gMfpyneykIbcgREOoFUsIwLOHahqczQOtV2El/6T1J3PgZGVawwO9otZuCxogw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1654030212; a=rsa-sha256; cv=none; b=viOrStEd9DV+E4vOnvQPO27GeJv3zu7NFFj/YPb6c0SpRCG+hZxfd3muLeYHTdynClwCEE WCyDe2YXwkvjUfkAvLGvvwUIMdy0HAjxgFHQ+LEYFSCinIBEIEcIM0K2PPUhbvbnfhFFCH NugY0+eUp7ruIzlHtovjgYqz5uEjQNsuoWfkvTI6QaeYWJLQYKZOd9O2BgEiRUH/HVJzdt n/oBG610ONEhpg0rNJ/XuTG28w9sjdM8qHjDUzzc5rEFMhS+yCMaosvD1TpKXP7R1RZt6Y kgrkD/RRBPH8Ml1KSPUyCvTH87eQSnDQsT7EA4KZupD44w1zYoNXzExHeRs2gg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by mjg: URL: https://cgit.FreeBSD.org/src/commit/?id=6c92016aa685486f1445f632ac3f1af1385186af commit 6c92016aa685486f1445f632ac3f1af1385186af Author: Mateusz Guzik AuthorDate: 2022-05-31 20:00:37 +0000 Commit: Mateusz Guzik CommitDate: 2022-05-31 20:11:39 +0000 pf: fix a race against kif destruction in pf_test{,6} ifp kif was dereferenced prior to taking the lock and could have been nullified later. Reviewed by: kp Sponsored by: Rubicon Communications, LLC ("Netgate") Differential Revision: --- sys/netpfil/pf/pf.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index c613194ce9b5..56dab43a2810 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -6978,18 +6978,25 @@ pf_test(int dir, int pflags, struct ifnet *ifp, struct mbuf **m0, struct inpcb * if (!V_pf_status.running) return (PF_PASS); + PF_RULES_RLOCK(); + kif = (struct pfi_kkif *)ifp->if_pf_kif; - if (kif == NULL) { + if (__predict_false(kif == NULL)) { DPFPRINTF(PF_DEBUG_URGENT, ("pf_test: kif == NULL, if_xname %s\n", ifp->if_xname)); + PF_RULES_RUNLOCK(); return (PF_DROP); } - if (kif->pfik_flags & PFI_IFLAG_SKIP) + if (kif->pfik_flags & PFI_IFLAG_SKIP) { + PF_RULES_RUNLOCK(); return (PF_PASS); + } - if (m->m_flags & M_SKIP_FIREWALL) + if (m->m_flags & M_SKIP_FIREWALL) { + PF_RULES_RUNLOCK(); return (PF_PASS); + } memset(&pd, 0, sizeof(pd)); pd.pf_mtag = pf_find_mtag(m); @@ -7000,10 +7007,12 @@ pf_test(int dir, int pflags, struct ifnet *ifp, struct mbuf **m0, struct inpcb * ifp = ifnet_byindexgen(pd.pf_mtag->if_index, pd.pf_mtag->if_idxgen); if (ifp == NULL || ifp->if_flags & IFF_DYING) { + PF_RULES_RUNLOCK(); m_freem(*m0); *m0 = NULL; return (PF_PASS); } + PF_RULES_RUNLOCK(); (ifp->if_output)(ifp, m, sintosa(&pd.pf_mtag->dst), NULL); *m0 = NULL; return (PF_PASS); @@ -7023,12 +7032,11 @@ pf_test(int dir, int pflags, struct ifnet *ifp, struct mbuf **m0, struct inpcb * /* But only once. We may see the packet multiple times (e.g. * PFIL_IN/PFIL_OUT). */ pd.pf_mtag->flags &= ~PF_TAG_DUMMYNET; + PF_RULES_RUNLOCK(); return (PF_PASS); } - PF_RULES_RLOCK(); - if (__predict_false(ip_divert_ptr != NULL) && ((ipfwtag = m_tag_locate(m, MTAG_IPFW_RULE, 0, NULL)) != NULL)) { struct ipfw_rule_ref *rr = (struct ipfw_rule_ref *)(ipfwtag+1); @@ -7468,17 +7476,24 @@ pf_test6(int dir, int pflags, struct ifnet *ifp, struct mbuf **m0, struct inpcb if (!V_pf_status.running) return (PF_PASS); + PF_RULES_RLOCK(); + kif = (struct pfi_kkif *)ifp->if_pf_kif; - if (kif == NULL) { + if (__predict_false(kif == NULL)) { DPFPRINTF(PF_DEBUG_URGENT, ("pf_test6: kif == NULL, if_xname %s\n", ifp->if_xname)); + PF_RULES_RUNLOCK(); return (PF_DROP); } - if (kif->pfik_flags & PFI_IFLAG_SKIP) + if (kif->pfik_flags & PFI_IFLAG_SKIP) { + PF_RULES_RUNLOCK(); return (PF_PASS); + } - if (m->m_flags & M_SKIP_FIREWALL) + if (m->m_flags & M_SKIP_FIREWALL) { + PF_RULES_RUNLOCK(); return (PF_PASS); + } memset(&pd, 0, sizeof(pd)); pd.pf_mtag = pf_find_mtag(m); @@ -7489,10 +7504,12 @@ pf_test6(int dir, int pflags, struct ifnet *ifp, struct mbuf **m0, struct inpcb ifp = ifnet_byindexgen(pd.pf_mtag->if_index, pd.pf_mtag->if_idxgen); if (ifp == NULL || ifp->if_flags & IFF_DYING) { + PF_RULES_RUNLOCK(); m_freem(*m0); *m0 = NULL; return (PF_PASS); } + PF_RULES_RUNLOCK(); nd6_output_ifp(ifp, ifp, m, (struct sockaddr_in6 *)&pd.pf_mtag->dst, NULL); *m0 = NULL; @@ -7510,11 +7527,10 @@ pf_test6(int dir, int pflags, struct ifnet *ifp, struct mbuf **m0, struct inpcb /* Dummynet re-injects packets after they've * completed their delay. We've already * processed them, so pass unconditionally. */ + PF_RULES_RUNLOCK(); return (PF_PASS); } - PF_RULES_RLOCK(); - /* We do IP header normalization and packet reassembly here */ if (pf_normalize_ip6(m0, dir, kif, &reason, &pd) != PF_PASS) { action = PF_DROP;