From d5072c26da8041c2b67235f7169daab91e70e844 Mon Sep 17 00:00:00 2001 From: bluhm Date: Sat, 23 Oct 2021 22:19:37 +0000 Subject: [PATCH] There is an m_pullup() down in AH input. As it may free or change the mbuf, the callers must be careful. Although there is no bug, use the common pattern to handle this. Pass down an mbuf pointer mp and let m_pullup() update the pointer in all callers. It looks like the tcp signature functions should not be called. Avoid an mbuf leak and return an error. OK mvs@ --- sys/net/if_bridge.c | 4 +-- sys/netinet/ip_ah.c | 70 ++++++++++++++++++++------------------- sys/netinet/ip_esp.c | 7 ++-- sys/netinet/ip_ipcomp.c | 28 +++++++++------- sys/netinet/ip_ipip.c | 6 ++-- sys/netinet/ip_ipsp.h | 16 ++++----- sys/netinet/ipsec_input.c | 21 ++++++------ sys/netinet/tcp_subr.c | 9 +++-- sys/netinet/udp_usrreq.c | 4 +-- 9 files changed, 88 insertions(+), 77 deletions(-) diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c index dfd19d1432c..cd9b0fa9a5a 100644 --- a/sys/net/if_bridge.c +++ b/sys/net/if_bridge.c @@ -1,4 +1,4 @@ -/* $OpenBSD: if_bridge.c,v 1.356 2021/07/07 20:19:01 sashan Exp $ */ +/* $OpenBSD: if_bridge.c,v 1.357 2021/10/23 22:19:37 bluhm Exp $ */ /* * Copyright (c) 1999, 2000 Jason L. Wright (jason@thought.net) @@ -1575,7 +1575,7 @@ bridge_ipsec(struct ifnet *ifp, struct ether_header *eh, int hassnap, tdb->tdb_soft_first_use); } - (*(tdb->tdb_xform->xf_input))(m, tdb, hlen, off); + (*(tdb->tdb_xform->xf_input))(&m, tdb, hlen, off); return (1); } else { skiplookup: diff --git a/sys/netinet/ip_ah.c b/sys/netinet/ip_ah.c index e64026d6392..ca347c5aba9 100644 --- a/sys/netinet/ip_ah.c +++ b/sys/netinet/ip_ah.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_ah.c,v 1.159 2021/10/23 15:42:35 tobhe Exp $ */ +/* $OpenBSD: ip_ah.c,v 1.160 2021/10/23 22:19:37 bluhm Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr) and @@ -197,9 +197,9 @@ ah_zeroize(struct tdb *tdbp) * Massage IPv4/IPv6 headers for AH processing. */ int -ah_massage_headers(struct mbuf **m0, int af, int skip, int alg, int out) +ah_massage_headers(struct mbuf **mp, int af, int skip, int alg, int out) { - struct mbuf *m = *m0; + struct mbuf *m = *mp; unsigned char *ptr; int off, count; struct ip *ip; @@ -216,11 +216,12 @@ ah_massage_headers(struct mbuf **m0, int af, int skip, int alg, int out) * and option processing -- just make sure they're in * contiguous memory. */ - *m0 = m = m_pullup(m, skip); + m = *mp = m_pullup(m, skip); if (m == NULL) { DPRINTF("m_pullup() failed"); ahstat_inc(ahs_hdrops); - return ENOBUFS; + error = ENOBUFS; + goto drop; } /* Fix the IP header */ @@ -240,8 +241,8 @@ ah_massage_headers(struct mbuf **m0, int af, int skip, int alg, int out) "for option %d", ptr[off]); ahstat_inc(ahs_hdrops); - m_freem(m); - return EINVAL; + error = EINVAL; + goto drop; } switch (ptr[off]) { @@ -264,8 +265,8 @@ ah_massage_headers(struct mbuf **m0, int af, int skip, int alg, int out) "for option %d", ptr[off]); ahstat_inc(ahs_hdrops); - m_freem(m); - return EINVAL; + error = EINVAL; + goto drop; } off += ptr[off + 1]; @@ -279,8 +280,8 @@ ah_massage_headers(struct mbuf **m0, int af, int skip, int alg, int out) "for option %d", ptr[off]); ahstat_inc(ahs_hdrops); - m_freem(m); - return EINVAL; + error = EINVAL; + goto drop; } /* @@ -307,8 +308,8 @@ ah_massage_headers(struct mbuf **m0, int af, int skip, int alg, int out) "for option %d", ptr[off]); ahstat_inc(ahs_hdrops); - m_freem(m); - return EINVAL; + error = EINVAL; + goto drop; } /* Zeroize all other options. */ @@ -322,8 +323,8 @@ ah_massage_headers(struct mbuf **m0, int af, int skip, int alg, int out) if (off > skip) { DPRINTF("malformed IPv4 options header"); ahstat_inc(ahs_hdrops); - m_freem(m); - return EINVAL; + error = EINVAL; + goto drop; } } @@ -338,8 +339,8 @@ ah_massage_headers(struct mbuf **m0, int af, int skip, int alg, int out) if (ip6.ip6_plen == 0) { DPRINTF("unsupported IPv6 jumbogram"); ahstat_inc(ahs_hdrops); - m_freem(m); - return EMSGSIZE; + error = EMSGSIZE; + goto drop; } ip6.ip6_flow = 0; @@ -359,8 +360,7 @@ ah_massage_headers(struct mbuf **m0, int af, int skip, int alg, int out) if (error) { DPRINTF("m_copyback no memory"); ahstat_inc(ahs_hdrops); - m_freem(m); - return error; + goto drop; } /* Let's deal with the remaining headers (if any). */ @@ -372,8 +372,8 @@ ah_massage_headers(struct mbuf **m0, int af, int skip, int alg, int out) DPRINTF("failed to allocate " "memory for IPv6 headers"); ahstat_inc(ahs_hdrops); - m_freem(m); - return ENOBUFS; + error = ENOBUFS; + goto drop; } /* @@ -478,8 +478,7 @@ ah_massage_headers(struct mbuf **m0, int af, int skip, int alg, int out) if (alloc) free(ptr, M_XDATA, 0); ahstat_inc(ahs_hdrops); - m_freem(m); - return error; + goto drop; } rh0->ip6r0_segleft = 0; } @@ -492,8 +491,8 @@ error6: if (alloc) free(ptr, M_XDATA, 0); ahstat_inc(ahs_hdrops); - m_freem(m); - return EINVAL; + error = EINVAL; + goto drop; } /* Advance. */ @@ -508,8 +507,7 @@ error6: free(ptr, M_XDATA, 0); if (error) { ahstat_inc(ahs_hdrops); - m_freem(m); - return error; + goto drop; } } @@ -518,6 +516,10 @@ error6: } return 0; + + drop: + m_freemp(mp); + return error; } /* @@ -525,9 +527,10 @@ error6: * passes authentication. */ int -ah_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) +ah_input(struct mbuf **mp, struct tdb *tdb, int skip, int protoff) { const struct auth_hash *ahx = tdb->tdb_authalgxform; + struct mbuf *m = *mp; struct tdb_crypto *tc = NULL; u_int32_t btsx, esn; u_int8_t hl; @@ -674,13 +677,12 @@ ah_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) m_copyback(m, skip + rplen, ahx->authsize, ipseczeroes, M_NOWAIT); /* "Massage" the packet headers for crypto processing. */ - error = ah_massage_headers(&m, tdb->tdb_dst.sa.sa_family, skip, + error = ah_massage_headers(mp, tdb->tdb_dst.sa.sa_family, skip, ahx->type, 0); - if (error) { - /* mbuf was freed by callee. */ - m = NULL; + /* callee may change or free mbuf */ + m = *mp; + if (error) goto drop; - } /* Crypto operation descriptor. */ crp->crp_ilen = m->m_pkthdr.len; /* Total input length. */ @@ -728,7 +730,7 @@ ah_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) return 0; drop: - m_freem(m); + m_freemp(mp); crypto_freereq(crp); free(tc, M_XDATA, 0); return error; diff --git a/sys/netinet/ip_esp.c b/sys/netinet/ip_esp.c index 391cfe0f4f5..ed287a15fa8 100644 --- a/sys/netinet/ip_esp.c +++ b/sys/netinet/ip_esp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_esp.c,v 1.178 2021/10/23 15:42:35 tobhe Exp $ */ +/* $OpenBSD: ip_esp.c,v 1.179 2021/10/23 22:19:37 bluhm Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr) and @@ -340,10 +340,11 @@ esp_zeroize(struct tdb *tdbp) * ESP input processing, called (eventually) through the protocol switch. */ int -esp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) +esp_input(struct mbuf **mp, struct tdb *tdb, int skip, int protoff) { const struct auth_hash *esph = tdb->tdb_authalgxform; const struct enc_xform *espx = tdb->tdb_encalgxform; + struct mbuf *m = *mp; struct cryptodesc *crde = NULL, *crda = NULL; struct cryptop *crp = NULL; struct tdb_crypto *tc = NULL; @@ -554,7 +555,7 @@ esp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) return 0; drop: - m_freem(m); + m_freemp(mp); crypto_freereq(crp); free(tc, M_XDATA, 0); return error; diff --git a/sys/netinet/ip_ipcomp.c b/sys/netinet/ip_ipcomp.c index 4a2a52f42af..7efa768b85e 100644 --- a/sys/netinet/ip_ipcomp.c +++ b/sys/netinet/ip_ipcomp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_ipcomp.c,v 1.81 2021/10/23 22:00:51 bluhm Exp $ */ +/* $OpenBSD: ip_ipcomp.c,v 1.82 2021/10/23 22:19:37 bluhm Exp $ */ /* * Copyright (c) 2001 Jean-Jacques Bernard-Gundol (jj@wabbitt.org) @@ -131,10 +131,11 @@ ipcomp_zeroize(struct tdb *tdbp) * ipcomp_input() gets called to uncompress an input packet */ int -ipcomp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) +ipcomp_input(struct mbuf **mp, struct tdb *tdb, int skip, int protoff) { const struct comp_algo *ipcompx = tdb->tdb_compalgxform; - struct tdb_crypto *tc; + struct mbuf *m = *mp; + struct tdb_crypto *tc = NULL; int hlen, error, clen; struct cryptodesc *crdc = NULL; @@ -145,19 +146,18 @@ ipcomp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) /* Get crypto descriptors */ crp = crypto_getreq(1); if (crp == NULL) { - m_freem(m); DPRINTF("failed to acquire crypto descriptors"); ipcompstat_inc(ipcomps_crypto); - return ENOBUFS; + error = ENOBUFS; + goto drop; } /* Get IPsec-specific opaque pointer */ tc = malloc(sizeof(*tc), M_XDATA, M_NOWAIT | M_ZERO); if (tc == NULL) { - m_freem(m); - crypto_freereq(crp); DPRINTF("failed to allocate tdb_crypto"); ipcompstat_inc(ipcomps_crypto); - return ENOBUFS; + error = ENOBUFS; + goto drop; } crdc = &crp->crp_desc[0]; @@ -194,10 +194,8 @@ ipcomp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) if (crp->crp_etype) { DPRINTF("crypto error %d", crp->crp_etype); ipsecstat_inc(ipsec_noxform); - free(tc, M_XDATA, 0); - m_freem(m); - crypto_freereq(crp); - return crp->crp_etype; + error = crp->crp_etype; + goto drop; } clen = crp->crp_olen; @@ -212,6 +210,12 @@ ipcomp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) } return 0; + + drop: + m_freemp(mp); + crypto_freereq(crp); + free(tc, M_XDATA, 0); + return error; } int diff --git a/sys/netinet/ip_ipip.c b/sys/netinet/ip_ipip.c index 6a4ef5929a6..9bcde8d533d 100644 --- a/sys/netinet/ip_ipip.c +++ b/sys/netinet/ip_ipip.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_ipip.c,v 1.96 2021/10/22 15:44:20 bluhm Exp $ */ +/* $OpenBSD: ip_ipip.c,v 1.97 2021/10/23 22:19:37 bluhm Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr) and @@ -557,11 +557,11 @@ ipe4_zeroize(struct tdb *tdbp) } int -ipe4_input(struct mbuf *m, struct tdb *tdb, int hlen, int proto) +ipe4_input(struct mbuf **mp, struct tdb *tdb, int hlen, int proto) { /* This is a rather serious mistake, so no conditional printing. */ printf("%s: should never be called\n", __func__); - m_freem(m); + m_freemp(mp); return EINVAL; } #endif /* IPSEC */ diff --git a/sys/netinet/ip_ipsp.h b/sys/netinet/ip_ipsp.h index 55452ca25c5..5703fe98ba4 100644 --- a/sys/netinet/ip_ipsp.h +++ b/sys/netinet/ip_ipsp.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_ipsp.h,v 1.211 2021/10/23 15:42:35 tobhe Exp $ */ +/* $OpenBSD: ip_ipsp.h,v 1.212 2021/10/23 22:19:37 bluhm Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr), @@ -479,7 +479,7 @@ struct xformsw { int (*xf_init)(struct tdb *, const struct xformsw *, struct ipsecinit *); int (*xf_zeroize)(struct tdb *); /* termination */ - int (*xf_input)(struct mbuf *, struct tdb *, int, int); + int (*xf_input)(struct mbuf **, struct tdb *, int, int); int (*xf_output)(struct mbuf *, struct tdb *, int, int); }; @@ -564,13 +564,13 @@ int tdb_walk(u_int, int (*)(struct tdb *, void *, int), void *); int ipe4_attach(void); int ipe4_init(struct tdb *, const struct xformsw *, struct ipsecinit *); int ipe4_zeroize(struct tdb *); -int ipe4_input(struct mbuf *, struct tdb *, int, int); +int ipe4_input(struct mbuf **, struct tdb *, int, int); /* XF_AH */ int ah_attach(void); int ah_init(struct tdb *, const struct xformsw *, struct ipsecinit *); int ah_zeroize(struct tdb *); -int ah_input(struct mbuf *, struct tdb *, int, int); +int ah_input(struct mbuf **, struct tdb *, int, int); int ah_input_cb(struct tdb *, struct tdb_crypto *, struct mbuf *, int); int ah_output(struct mbuf *, struct tdb *, int, int); int ah_output_cb(struct tdb *, struct tdb_crypto *, struct mbuf *, int, @@ -589,7 +589,7 @@ int ah6_input(struct mbuf **, int *, int, int); int esp_attach(void); int esp_init(struct tdb *, const struct xformsw *, struct ipsecinit *); int esp_zeroize(struct tdb *); -int esp_input(struct mbuf *, struct tdb *, int, int); +int esp_input(struct mbuf **, struct tdb *, int, int); int esp_input_cb(struct tdb *, struct tdb_crypto *, struct mbuf *, int); int esp_output(struct mbuf *, struct tdb *, int, int); int esp_output_cb(struct tdb *, struct tdb_crypto *, struct mbuf *, int, @@ -607,7 +607,7 @@ int esp6_input(struct mbuf **, int *, int, int); int ipcomp_attach(void); int ipcomp_init(struct tdb *, const struct xformsw *, struct ipsecinit *); int ipcomp_zeroize(struct tdb *); -int ipcomp_input(struct mbuf *, struct tdb *, int, int); +int ipcomp_input(struct mbuf **, struct tdb *, int, int); int ipcomp_input_cb(struct tdb *, struct tdb_crypto *, struct mbuf *, int); int ipcomp_output(struct mbuf *, struct tdb *, int, int); int ipcomp_output_cb(struct tdb *, struct tdb_crypto *, struct mbuf *, int, @@ -623,7 +623,7 @@ int tcp_signature_tdb_attach(void); int tcp_signature_tdb_init(struct tdb *, const struct xformsw *, struct ipsecinit *); int tcp_signature_tdb_zeroize(struct tdb *); -int tcp_signature_tdb_input(struct mbuf *, struct tdb *, int, int); +int tcp_signature_tdb_input(struct mbuf **, struct tdb *, int, int); int tcp_signature_tdb_output(struct mbuf *, struct tdb *, int, int); /* Replay window */ @@ -647,7 +647,7 @@ void ipsp_ids_free(struct ipsec_ids *); void ipsp_init(void); void ipsec_init(void); int ipsec_sysctl(int *, u_int, void *, size_t *, void *, size_t); -int ipsec_common_input(struct mbuf *, int, int, int, int, int); +int ipsec_common_input(struct mbuf **, int, int, int, int, int); int ipsec_common_input_cb(struct mbuf *, struct tdb *, int, int); int ipsec_delete_policy(struct ipsec_policy *); ssize_t ipsec_hdrsz(struct tdb *); diff --git a/sys/netinet/ipsec_input.c b/sys/netinet/ipsec_input.c index 12b845087d5..b5cea182417 100644 --- a/sys/netinet/ipsec_input.c +++ b/sys/netinet/ipsec_input.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ipsec_input.c,v 1.186 2021/10/23 15:42:35 tobhe Exp $ */ +/* $OpenBSD: ipsec_input.c,v 1.187 2021/10/23 22:19:37 bluhm Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr) and @@ -176,7 +176,7 @@ ipsec_init(void) * filtering). */ int -ipsec_common_input(struct mbuf *m, int skip, int protoff, int af, int sproto, +ipsec_common_input(struct mbuf **mp, int skip, int protoff, int af, int sproto, int udpencap) { #define IPSEC_ISTAT(x,y,z) do { \ @@ -188,6 +188,7 @@ ipsec_common_input(struct mbuf *m, int skip, int protoff, int af, int sproto, ipcompstat_inc(z); \ } while (0) + struct mbuf *m = *mp; union sockaddr_union dst_address; struct tdb *tdbp = NULL; struct ifnet *encif; @@ -351,7 +352,7 @@ ipsec_common_input(struct mbuf *m, int skip, int protoff, int af, int sproto, * Call appropriate transform and return -- callback takes care of * everything else. */ - error = (*(tdbp->tdb_xform->xf_input))(m, tdbp, skip, protoff); + error = (*(tdbp->tdb_xform->xf_input))(mp, tdbp, skip, protoff); if (error) { ipsecstat_inc(ipsec_idrops); tdbp->tdb_idrops++; @@ -359,7 +360,7 @@ ipsec_common_input(struct mbuf *m, int skip, int protoff, int af, int sproto, return error; drop: - m_freem(m); + m_freemp(mp); ipsecstat_inc(ipsec_idrops); if (tdbp != NULL) tdbp->tdb_idrops++; @@ -801,7 +802,7 @@ ah4_input(struct mbuf **mp, int *offp, int proto, int af) !ah_enable) return rip_input(mp, offp, proto, af); - ipsec_common_input(*mp, *offp, offsetof(struct ip, ip_p), AF_INET, + ipsec_common_input(mp, *offp, offsetof(struct ip, ip_p), AF_INET, proto, 0); return IPPROTO_DONE; } @@ -827,7 +828,7 @@ esp4_input(struct mbuf **mp, int *offp, int proto, int af) !esp_enable) return rip_input(mp, offp, proto, af); - ipsec_common_input(*mp, *offp, offsetof(struct ip, ip_p), AF_INET, + ipsec_common_input(mp, *offp, offsetof(struct ip, ip_p), AF_INET, proto, 0); return IPPROTO_DONE; } @@ -843,7 +844,7 @@ ipcomp4_input(struct mbuf **mp, int *offp, int proto, int af) !ipcomp_enable) return rip_input(mp, offp, proto, af); - ipsec_common_input(*mp, *offp, offsetof(struct ip, ip_p), AF_INET, + ipsec_common_input(mp, *offp, offsetof(struct ip, ip_p), AF_INET, proto, 0); return IPPROTO_DONE; } @@ -1020,7 +1021,7 @@ ah6_input(struct mbuf **mp, int *offp, int proto, int af) } protoff += offsetof(struct ip6_ext, ip6e_nxt); } - ipsec_common_input(*mp, *offp, protoff, AF_INET6, proto, 0); + ipsec_common_input(mp, *offp, protoff, AF_INET6, proto, 0); return IPPROTO_DONE; } @@ -1077,7 +1078,7 @@ esp6_input(struct mbuf **mp, int *offp, int proto, int af) } protoff += offsetof(struct ip6_ext, ip6e_nxt); } - ipsec_common_input(*mp, *offp, protoff, AF_INET6, proto, 0); + ipsec_common_input(mp, *offp, protoff, AF_INET6, proto, 0); return IPPROTO_DONE; } @@ -1135,7 +1136,7 @@ ipcomp6_input(struct mbuf **mp, int *offp, int proto, int af) protoff += offsetof(struct ip6_ext, ip6e_nxt); } - ipsec_common_input(*mp, *offp, protoff, AF_INET6, proto, 0); + ipsec_common_input(mp, *offp, protoff, AF_INET6, proto, 0); return IPPROTO_DONE; } #endif /* INET6 */ diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c index 87819a1a74b..c1349d84397 100644 --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tcp_subr.c,v 1.180 2021/10/13 14:36:31 bluhm Exp $ */ +/* $OpenBSD: tcp_subr.c,v 1.181 2021/10/23 22:19:37 bluhm Exp $ */ /* $NetBSD: tcp_subr.c,v 1.22 1996/02/13 23:44:00 christos Exp $ */ /* @@ -960,15 +960,18 @@ tcp_signature_tdb_zeroize(struct tdb *tdbp) } int -tcp_signature_tdb_input(struct mbuf *m, struct tdb *tdbp, int skip, int protoff) +tcp_signature_tdb_input(struct mbuf **mp, struct tdb *tdbp, int skip, + int protoff) { - return (0); + m_freemp(mp); + return (EINVAL); } int tcp_signature_tdb_output(struct mbuf *m, struct tdb *tdbp, int skip, int protoff) { + m_freem(m); return (EINVAL); } diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c index 19752d29fd8..e49cc2fa891 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -1,4 +1,4 @@ -/* $OpenBSD: udp_usrreq.c,v 1.262 2020/08/22 17:54:57 gnezdo Exp $ */ +/* $OpenBSD: udp_usrreq.c,v 1.263 2021/10/23 22:19:37 bluhm Exp $ */ /* $NetBSD: udp_usrreq.c,v 1.28 1996/03/16 23:54:03 christos Exp $ */ /* @@ -305,7 +305,7 @@ udp_input(struct mbuf **mp, int *offp, int proto, int af) espstat_inc(esps_udpencin); protoff = af == AF_INET ? offsetof(struct ip, ip_p) : offsetof(struct ip6_hdr, ip6_nxt); - ipsec_common_input(m, skip, protoff, + ipsec_common_input(mp, skip, protoff, af, IPPROTO_ESP, 1); return IPPROTO_DONE; } -- 2.20.1