From nobody Fri Jan 17 16:02:12 2025 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 4YZPd10GMhz5l8TQ; Fri, 17 Jan 2025 16:02:13 +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 4YZPd06mFHz46xD; Fri, 17 Jan 2025 16:02:12 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737129733; 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=q5O5BSI4Ff2R3FT5sLgjIqQSjUraGXa5K1PGBkziQTw=; b=GYSUtniQBshUoWqDA564Guo+tA4tCzoqv3bPj88CnUpuQWKQbCtC7KXsbEzbkA26UEo8to mzPObrxhLooHdc2/0HcE2aFhYQW9m17PFEbDQzU4tBilgZq1LjAXK9/Cz1+0yRsF7wpfOU 9Cz5ih4u4Edq1nKEC6fcZa0jc2WiSnKPXHfx35x3vOMKjkB72Uz/79jSP7r4xJbh3lHXzY wMM35cIOI/50OXWyEd75CmASru2+qt2tZbNuTa5sUvJWeHTcqZ+h+epQyLrZ6k9cfYaouq ZvEX2LYafd9E0J+T3Sec47U0v+x2HEalLGbC30dg9o7R204rcy1/KjNhtfmsNQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737129733; 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=q5O5BSI4Ff2R3FT5sLgjIqQSjUraGXa5K1PGBkziQTw=; b=poUw+ko/GpdyCUwtVIFttQtCiSqUX0TN8M9tm8rFewuAKnlW61a2oC0VzndMONZ3cZhVdI SkfhIVbSdppk3XRyhoa1/EEVHBfwYlpsdbgEE18TLwwO1hhMZfLFtdUX8IGd6gKS622WpS m7yAFwUcBew8CQ+enjfA4sRtG7eSK4xCBaiR0iG8Wgs9WJfyqub9mJQpT5pEvRuXcuSLy3 q4ZGRtQcAhlwEy0XnrOh2cTAapuaFWGVXtcOARpDkc1GM6FzOyB4UurISOlzmb9JDal37T xj/JC0y+VYLE3WvlF60uM5qM1Oy/0kbLWKALBm56xz+r0N8AbKb+6fvT5BYFcg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1737129733; a=rsa-sha256; cv=none; b=jR+/xcglr4aFopzzDiV8+25vYNEIFkd/TktV/dB8TIRe6YymxKIDdDd+gFuGnmvSHTh0LD AaVCQSVD0EcvGnFBeFgdzfhTXK1YVehNCvGu7UWmZdi7WgbDp+Pr849i20c5iMWEiw3S5T b4p/n/xFG1KWDvaTKNLRGUtjsk7L66EyRp68e/0n24Kipz2+5TMW8u0NXzVmsoEvk5lS9W uWbrCOaVQaYKwxz4yhJAVOg96U3+1YyqpF13evsLQqE8n2WUZaLrt+SkRicERdWNDKgo2m muCYpWQfifs7IlGnlKtLT93+Kr66kj+miyfcLhkpdpeyI1IsMRnjQKBRYMx15w== ARC-Authentication-Results: i=1; mx1.freebsd.org; none 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 4YZPd067tYz8Lb; Fri, 17 Jan 2025 16:02:12 +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 50HG2CqC065698; Fri, 17 Jan 2025 16:02:12 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 50HG2CYU065695; Fri, 17 Jan 2025 16:02:12 GMT (envelope-from git) Date: Fri, 17 Jan 2025 16:02:12 GMT Message-Id: <202501171602.50HG2CYU065695@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: 5d28f4cab8d5 - main - pf: clean up mbuf passing for reassembly 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: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 5d28f4cab8d5919aba1365e885a91a96c0655b59 Auto-Submitted: auto-generated The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=5d28f4cab8d5919aba1365e885a91a96c0655b59 commit 5d28f4cab8d5919aba1365e885a91a96c0655b59 Author: Kristof Provost AuthorDate: 2025-01-06 06:56:24 +0000 Commit: Kristof Provost CommitDate: 2025-01-17 16:00:08 +0000 pf: clean up mbuf passing for reassembly When we call pf_normalize_ip() or pf_normalize_ip6() we passed the mbuf twice. Once as m0, and once inside the struct pf_pdesc. Remove the former to avoid confusion when we free *m0, but don't update pd->m. This could lead to use-after-free errors e.g. if reassembly failed. PR: 283705 Reported by: Yichen Chai , Zhuo Ying Jiang Li MFC after: 3 days Sponsored by: Rubicon Communications, LLC ("Netgate") --- sys/net/pfvar.h | 4 ++-- sys/netpfil/pf/pf.c | 10 ++++++---- sys/netpfil/pf/pf_norm.c | 17 +++++------------ 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index 6c9d75bb1365..6e9418f59aef 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -2390,11 +2390,11 @@ int pf_test(sa_family_t, int, int, struct ifnet *, struct mbuf **, struct inpcb struct pf_rule_actions *); #endif #ifdef INET -int pf_normalize_ip(struct mbuf **, u_short *, struct pf_pdesc *); +int pf_normalize_ip(u_short *, struct pf_pdesc *); #endif /* INET */ #ifdef INET6 -int pf_normalize_ip6(struct mbuf **, int, u_short *, struct pf_pdesc *); +int pf_normalize_ip6(int, u_short *, struct pf_pdesc *); void pf_poolmask(struct pf_addr *, struct pf_addr*, struct pf_addr *, struct pf_addr *, sa_family_t); void pf_addr_inc(struct pf_addr *, sa_family_t); diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 07b7aa543dbd..3b9c71296a2b 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -9918,12 +9918,13 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0, return (-1); } - if (pf_normalize_ip(m0, reason, pd) != PF_PASS) { + if (pf_normalize_ip(reason, pd) != PF_PASS) { /* We do IP header normalization and packet reassembly here */ + *m0 = pd->m; *action = PF_DROP; return (-1); } - pd->m = *m0; + *m0 = pd->m; h = mtod(pd->m, struct ip *); pd->off = h->ip_hl << 2; @@ -9994,12 +9995,13 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0, } /* We do IP header normalization and packet reassembly here */ - if (pf_normalize_ip6(m0, pd->fragoff, reason, pd) != + if (pf_normalize_ip6(pd->fragoff, reason, pd) != PF_PASS) { + *m0 = pd->m; *action = PF_DROP; return (-1); } - pd->m = *m0; + *m0 = pd->m; if (pd->m == NULL) { /* packet sits in reassembly queue, no error */ *action = PF_PASS; diff --git a/sys/netpfil/pf/pf_norm.c b/sys/netpfil/pf/pf_norm.c index c9a7f7d2df04..39afde04a6d1 100644 --- a/sys/netpfil/pf/pf_norm.c +++ b/sys/netpfil/pf/pf_norm.c @@ -1093,11 +1093,10 @@ pf_refragment6(struct ifnet *ifp, struct mbuf **m0, struct m_tag *mtag, #ifdef INET int -pf_normalize_ip(struct mbuf **m0, u_short *reason, - struct pf_pdesc *pd) +pf_normalize_ip(u_short *reason, struct pf_pdesc *pd) { struct pf_krule *r; - struct ip *h = mtod(*m0, struct ip *); + struct ip *h = mtod(pd->m, struct ip *); int mff = (ntohs(h->ip_off) & IP_MF); int hlen = h->ip_hl << 2; u_int16_t fragoff = (ntohs(h->ip_off) & IP_OFFMASK) << 3; @@ -1109,8 +1108,6 @@ pf_normalize_ip(struct mbuf **m0, u_short *reason, PF_RULES_RASSERT(); - MPASS(pd->m == *m0); - r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_SCRUB].active.ptr); /* * Check if there are any scrub rules, matching or not. @@ -1219,13 +1216,12 @@ pf_normalize_ip(struct mbuf **m0, u_short *reason, * Might return a completely reassembled mbuf, or NULL */ PF_FRAG_LOCK(); DPFPRINTF(("reass frag %d @ %d-%d\n", h->ip_id, fragoff, max)); - verdict = pf_reassemble(m0, pd->dir, reason); + verdict = pf_reassemble(&pd->m, pd->dir, reason); PF_FRAG_UNLOCK(); if (verdict != PF_PASS) return (PF_DROP); - pd->m = *m0; if (pd->m == NULL) return (PF_DROP); @@ -1257,7 +1253,7 @@ pf_normalize_ip(struct mbuf **m0, u_short *reason, #ifdef INET6 int -pf_normalize_ip6(struct mbuf **m0, int off, u_short *reason, +pf_normalize_ip6(int off, u_short *reason, struct pf_pdesc *pd) { struct pf_krule *r; @@ -1267,8 +1263,6 @@ pf_normalize_ip6(struct mbuf **m0, int off, u_short *reason, PF_RULES_RASSERT(); - pd->m = *m0; - r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_SCRUB].active.ptr); /* * Check if there are any scrub rules, matching or not. @@ -1323,9 +1317,8 @@ pf_normalize_ip6(struct mbuf **m0, int off, u_short *reason, if (pd->virtual_proto == PF_VPROTO_FRAGMENT) { /* Returns PF_DROP or *m0 is NULL or completely reassembled * mbuf. */ - if (pf_reassemble6(m0, &frag, off, pd->extoff, reason) != PF_PASS) + if (pf_reassemble6(&pd->m, &frag, off, pd->extoff, reason) != PF_PASS) return (PF_DROP); - pd->m = *m0; if (pd->m == NULL) return (PF_DROP); h = mtod(pd->m, struct ip6_hdr *);