Date: Sun, 3 Jan 2021 22:18:48 GMT From: Kristof Provost <kp@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 21745738a2b5 - stable/12 - pf: Fix unaligned checksum updates Message-ID: <202101032218.103MImmF020354@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=21745738a2b5662dfbe730b6338aa38e829cb0eb commit 21745738a2b5662dfbe730b6338aa38e829cb0eb Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2020-12-20 20:06:32 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2021-01-03 22:11:08 +0000 pf: Fix unaligned checksum updates The algorithm we use to update checksums only works correctly if the updated data is aligned on 16-bit boundaries (relative to the start of the packet). Import the OpenBSD fix for this issue. PR: 240416 Obtained from: OpenBSD MFC after: 1 week Reviewed by: tuexen (previous version) Differential Revision: https://reviews.freebsd.org/D27696 (cherry picked from commit c3f69af03ae7acc167cc1151f0c1ecc5e014ce4e) --- sys/net/pfvar.h | 5 +++ sys/netpfil/pf/pf.c | 81 +++++++++++++++++++++++++++++++++++++++--------- sys/netpfil/pf/pf_norm.c | 23 ++++++++++---- 3 files changed, 89 insertions(+), 20 deletions(-) diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index d0eb226ee41d..24faee5d45c6 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -330,6 +330,7 @@ extern struct sx pf_end_lock; (neg) \ ) +#define PF_ALGNMNT(off) (((off) % 2) == 0) struct pf_rule_uid { uid_t uid[2]; @@ -1727,6 +1728,10 @@ void pf_change_a(void *, u_int16_t *, u_int32_t, u_int8_t); void pf_change_proto_a(struct mbuf *, void *, u_int16_t *, u_int32_t, u_int8_t); void pf_change_tcp_a(struct mbuf *, void *, u_int16_t *, u_int32_t); +void pf_patch_16_unaligned(struct mbuf *, u_int16_t *, void *, u_int16_t, + bool, u_int8_t); +void pf_patch_32_unaligned(struct mbuf *, u_int16_t *, void *, u_int32_t, + bool, u_int8_t); void pf_send_deferred_syn(struct pf_state *); int pf_match_addr(u_int8_t, struct pf_addr *, struct pf_addr *, struct pf_addr *, sa_family_t); diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 693e45504745..84133039eb45 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -289,6 +289,8 @@ static void pf_print_state_parts(struct pf_state *, struct pf_state_key *, struct pf_state_key *); static int pf_addr_wrap_neq(struct pf_addr_wrap *, struct pf_addr_wrap *); +static void pf_patch_8(struct mbuf *, u_int16_t *, u_int8_t *, u_int8_t, + bool, u_int8_t); static struct pf_state *pf_find_state(struct pfi_kif *, struct pf_state_key_cmp *, u_int); static int pf_src_connlimit(struct pf_state **); @@ -2091,16 +2093,60 @@ pf_addr_wrap_neq(struct pf_addr_wrap *aw1, struct pf_addr_wrap *aw2) u_int16_t pf_cksum_fixup(u_int16_t cksum, u_int16_t old, u_int16_t new, u_int8_t udp) { - u_int32_t l; - - if (udp && !cksum) - return (0x0000); - l = cksum + old - new; - l = (l >> 16) + (l & 65535); - l = l & 65535; - if (udp && !l) - return (0xFFFF); - return (l); + u_int32_t x; + + x = cksum + old - new; + x = (x + (x >> 16)) & 0xffff; + + /* optimise: eliminate a branch when not udp */ + if (udp && cksum == 0x0000) + return cksum; + if (udp && x == 0x0000) + x = 0xffff; + + return (u_int16_t)(x); +} + +static void +pf_patch_8(struct mbuf *m, u_int16_t *cksum, u_int8_t *f, u_int8_t v, bool hi, + u_int8_t udp) +{ + u_int16_t old = htons(hi ? (*f << 8) : *f); + u_int16_t new = htons(hi ? ( v << 8) : v); + + if (*f == v) + return; + + *f = v; + + if (m->m_pkthdr.csum_flags & (CSUM_DELAY_DATA | CSUM_DELAY_DATA_IPV6)) + return; + + *cksum = pf_cksum_fixup(*cksum, old, new, udp); +} + +void +pf_patch_16_unaligned(struct mbuf *m, u_int16_t *cksum, void *f, u_int16_t v, + bool hi, u_int8_t udp) +{ + u_int8_t *fb = (u_int8_t *)f; + u_int8_t *vb = (u_int8_t *)&v; + + pf_patch_8(m, cksum, fb++, *vb++, hi, udp); + pf_patch_8(m, cksum, fb++, *vb++, !hi, udp); +} + +void +pf_patch_32_unaligned(struct mbuf *m, u_int16_t *cksum, void *f, u_int32_t v, + bool hi, u_int8_t udp) +{ + u_int8_t *fb = (u_int8_t *)f; + u_int8_t *vb = (u_int8_t *)&v; + + pf_patch_8(m, cksum, fb++, *vb++, hi, udp); + pf_patch_8(m, cksum, fb++, *vb++, !hi, udp); + pf_patch_8(m, cksum, fb++, *vb++, hi, udp); + pf_patch_8(m, cksum, fb++, *vb++, !hi, udp); } u_int16_t @@ -2327,6 +2373,7 @@ pf_modulate_sack(struct mbuf *m, int off, struct pf_pdesc *pd, return 0; while (hlen >= TCPOLEN_SACKLEN) { + size_t startoff = opt - opts; olen = opt[1]; switch (*opt) { case TCPOPT_EOL: /* FALLTHROUGH */ @@ -2341,10 +2388,16 @@ pf_modulate_sack(struct mbuf *m, int off, struct pf_pdesc *pd, for (i = 2; i + TCPOLEN_SACK <= olen; i += TCPOLEN_SACK) { memcpy(&sack, &opt[i], sizeof(sack)); - pf_change_proto_a(m, &sack.start, &th->th_sum, - htonl(ntohl(sack.start) - dst->seqdiff), 0); - pf_change_proto_a(m, &sack.end, &th->th_sum, - htonl(ntohl(sack.end) - dst->seqdiff), 0); + pf_patch_32_unaligned(m, + &th->th_sum, &sack.start, + htonl(ntohl(sack.start) - dst->seqdiff), + PF_ALGNMNT(startoff), + 0); + pf_patch_32_unaligned(m, &th->th_sum, + &sack.end, + htonl(ntohl(sack.end) - dst->seqdiff), + PF_ALGNMNT(startoff), + 0); memcpy(&opt[i], &sack, sizeof(sack)); } copyback = 1; diff --git a/sys/netpfil/pf/pf_norm.c b/sys/netpfil/pf/pf_norm.c index 7234c1def914..4bfdcd7898fe 100644 --- a/sys/netpfil/pf/pf_norm.c +++ b/sys/netpfil/pf/pf_norm.c @@ -1371,6 +1371,7 @@ pf_normalize_tcp_stateful(struct mbuf *m, int off, struct pf_pdesc *pd, u_int8_t *opt; int copyback = 0; int got_ts = 0; + size_t startoff; KASSERT((src->scrub || dst->scrub), ("%s: src->scrub && dst->scrub!", __func__)); @@ -1414,6 +1415,7 @@ pf_normalize_tcp_stateful(struct mbuf *m, int off, struct pf_pdesc *pd, opt = hdr + sizeof(struct tcphdr); hlen = (th->th_off << 2) - sizeof(struct tcphdr); while (hlen >= TCPOLEN_TIMESTAMP) { + startoff = opt - (hdr + sizeof(struct tcphdr)); switch (*opt) { case TCPOPT_EOL: /* FALLTHROUGH */ case TCPOPT_NOP: @@ -1443,10 +1445,12 @@ pf_normalize_tcp_stateful(struct mbuf *m, int off, struct pf_pdesc *pd, (src->scrub->pfss_flags & PFSS_TIMESTAMP)) { tsval = ntohl(tsval); - pf_change_proto_a(m, &opt[2], + pf_patch_32_unaligned(m, &th->th_sum, + &opt[2], htonl(tsval + src->scrub->pfss_ts_mod), + PF_ALGNMNT(startoff), 0); copyback = 1; } @@ -1459,8 +1463,11 @@ pf_normalize_tcp_stateful(struct mbuf *m, int off, struct pf_pdesc *pd, PFSS_TIMESTAMP)) { tsecr = ntohl(tsecr) - dst->scrub->pfss_ts_mod; - pf_change_proto_a(m, &opt[6], - &th->th_sum, htonl(tsecr), + pf_patch_32_unaligned(m, + &th->th_sum, + &opt[6], + htonl(tsecr), + PF_ALGNMNT(startoff), 0); copyback = 1; } @@ -1761,6 +1768,7 @@ pf_normalize_tcpopt(struct pf_rule *r, struct mbuf *m, struct tcphdr *th, int rewrite = 0; u_char opts[TCP_MAXOLEN]; u_char *optp = opts; + size_t startoff; thoff = th->th_off << 2; cnt = thoff - sizeof(struct tcphdr); @@ -1770,6 +1778,7 @@ pf_normalize_tcpopt(struct pf_rule *r, struct mbuf *m, struct tcphdr *th, return (rewrite); for (; cnt > 0; cnt -= optlen, optp += optlen) { + startoff = optp - opts; opt = optp[0]; if (opt == TCPOPT_EOL) break; @@ -1786,9 +1795,11 @@ pf_normalize_tcpopt(struct pf_rule *r, struct mbuf *m, struct tcphdr *th, case TCPOPT_MAXSEG: mss = (u_int16_t *)(optp + 2); if ((ntohs(*mss)) > r->max_mss) { - th->th_sum = pf_proto_cksum_fixup(m, - th->th_sum, *mss, htons(r->max_mss), 0); - *mss = htons(r->max_mss); + pf_patch_16_unaligned(m, + &th->th_sum, + mss, htons(r->max_mss), + PF_ALGNMNT(startoff), + 0); rewrite = 1; } break;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202101032218.103MImmF020354>