Date: Wed, 26 May 2021 01:50:06 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 544b5d24828e - stable/13 - Fix mbuf leaks in various pru_send implementations Message-ID: <202105260150.14Q1o66L042735@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=544b5d24828e249c84bc79b83ac6e9d4dbbc83ca commit 544b5d24828e249c84bc79b83ac6e9d4dbbc83ca Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2021-05-12 13:39:36 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2021-05-26 01:49:53 +0000 Fix mbuf leaks in various pru_send implementations The various protocol implementations are not very consistent about freeing mbufs in error paths. In general, all protocols must free both "m" and "control" upon an error, except if PRUS_NOTREADY is specified (this is only implemented by TCP and unix(4) and requires further work not handled in this diff), in which case "control" still must be freed. This diff plugs various leaks in the pru_send implementations. Reviewed by: tuexen Sponsored by: The FreeBSD Foundation (cherry picked from commit d8acd2681bcfb2ff7eb154df82f268b1cb191b4c) --- sys/kern/uipc_socket.c | 6 +++- sys/netinet/ip_divert.c | 2 ++ sys/netinet/raw_ip.c | 19 +++++++---- sys/netinet/tcp_usrreq.c | 77 ++++++++++++++++++--------------------------- sys/netinet/udp_usrreq.c | 1 + sys/netinet6/raw_ip6.c | 40 ++++++++++++----------- sys/netinet6/sctp6_usrreq.c | 25 +++++++++++++++ sys/netinet6/send.c | 3 ++ 8 files changed, 100 insertions(+), 73 deletions(-) diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c index 3436004b53a6..77ce876fd20f 100644 --- a/sys/kern/uipc_socket.c +++ b/sys/kern/uipc_socket.c @@ -3735,7 +3735,11 @@ pru_send_notsupp(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr, struct mbuf *control, struct thread *td) { - return EOPNOTSUPP; + if (control != NULL) + m_freem(control); + if ((flags & PRUS_NOTREADY) == 0) + m_freem(m); + return (EOPNOTSUPP); } int diff --git a/sys/netinet/ip_divert.c b/sys/netinet/ip_divert.c index f83a42cb36c9..77a4bfcd08ac 100644 --- a/sys/netinet/ip_divert.c +++ b/sys/netinet/ip_divert.c @@ -677,6 +677,8 @@ div_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam, if (m->m_len < sizeof (struct ip) && (m = m_pullup(m, sizeof (struct ip))) == NULL) { KMOD_IPSTAT_INC(ips_toosmall); + if (control != NULL) + m_freem(control); m_freem(m); return EINVAL; } diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c index 89ab6a6bbdad..1db73a6da68c 100644 --- a/sys/netinet/raw_ip.c +++ b/sys/netinet/raw_ip.c @@ -1077,13 +1077,18 @@ rip_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam, inp = sotoinpcb(so); KASSERT(inp != NULL, ("rip_send: inp == NULL")); + if (control != NULL) { + m_freem(control); + control = NULL; + } + /* * Note: 'dst' reads below are unlocked. */ if (so->so_state & SS_ISCONNECTED) { if (nam) { - m_freem(m); - return (EISCONN); + error = EISCONN; + goto release; } dst = inp->inp_faddr.s_addr; /* Unlocked read. */ } else { @@ -1094,13 +1099,15 @@ rip_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam, error = EAFNOSUPPORT; else if (nam->sa_len != sizeof(struct sockaddr_in)) error = EINVAL; - if (error != 0) { - m_freem(m); - return (error); - } + if (error != 0) + goto release; dst = ((struct sockaddr_in *)nam)->sin_addr.s_addr; } return (rip_output(m, so, dst)); + +release: + m_freem(m); + return (error); } #endif /* INET */ diff --git a/sys/netinet/tcp_usrreq.c b/sys/netinet/tcp_usrreq.c index 8c2a4c85d4d2..7132349dbba7 100644 --- a/sys/netinet/tcp_usrreq.c +++ b/sys/netinet/tcp_usrreq.c @@ -993,21 +993,31 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, if (inp->inp_flags & (INP_TIMEWAIT | INP_DROPPED)) { if (control) m_freem(control); + /* * In case of PRUS_NOTREADY, tcp_usr_ready() is responsible * for freeing memory. */ - if (m && (flags & PRUS_NOTREADY) == 0) + if ((flags & PRUS_NOTREADY) == 0) m_freem(m); error = ECONNRESET; goto out; } + if (control != NULL) { + /* TCP doesn't do control messages (rights, creds, etc) */ + if (control->m_len) { + m_freem(control); + m_freem(m); + error = EINVAL; + goto out; + } + m_freem(control); /* empty control, just free it */ + control = NULL; + } tp = intotcpcb(inp); if (flags & PRUS_OOB) { if ((error = tcp_pru_options_support(tp, PRUS_OOB)) != 0) { - if (control) - m_freem(control); - if (m && (flags & PRUS_NOTREADY) == 0) + if ((flags & PRUS_NOTREADY) == 0) m_freem(m); goto out; } @@ -1019,33 +1029,28 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, case AF_INET: sinp = (struct sockaddr_in *)nam; if (sinp->sin_len != sizeof(struct sockaddr_in)) { - if (m) - m_freem(m); + m_freem(m); error = EINVAL; goto out; } if ((inp->inp_vflag & INP_IPV6) != 0) { - if (m) - m_freem(m); + m_freem(m); error = EAFNOSUPPORT; goto out; } if (IN_MULTICAST(ntohl(sinp->sin_addr.s_addr))) { - if (m) - m_freem(m); + m_freem(m); error = EAFNOSUPPORT; goto out; } if (ntohl(sinp->sin_addr.s_addr) == INADDR_BROADCAST) { - if (m) - m_freem(m); + m_freem(m); error = EACCES; goto out; } if ((error = prison_remote_ip4(td->td_ucred, &sinp->sin_addr))) { - if (m) - m_freem(m); + m_freem(m); goto out; } #ifdef INET6 @@ -1060,20 +1065,17 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, sin6 = (struct sockaddr_in6 *)nam; if (sin6->sin6_len != sizeof(*sin6)) { - if (m) - m_freem(m); + m_freem(m); error = EINVAL; goto out; } if ((inp->inp_vflag & INP_IPV6PROTO) == 0) { - if (m != NULL) - m_freem(m); + m_freem(m); error = EAFNOSUPPORT; goto out; } if (IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr)) { - if (m) - m_freem(m); + m_freem(m); error = EAFNOSUPPORT; goto out; } @@ -1081,14 +1083,12 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, #ifdef INET if ((inp->inp_flags & IN6P_IPV6_V6ONLY) != 0) { error = EINVAL; - if (m) - m_freem(m); + m_freem(m); goto out; } if ((inp->inp_vflag & INP_IPV4) == 0) { error = EAFNOSUPPORT; - if (m) - m_freem(m); + m_freem(m); goto out; } restoreflags = true; @@ -1098,27 +1098,23 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, if (IN_MULTICAST( ntohl(sinp->sin_addr.s_addr))) { error = EAFNOSUPPORT; - if (m) - m_freem(m); + m_freem(m); goto out; } if ((error = prison_remote_ip4(td->td_ucred, &sinp->sin_addr))) { - if (m) - m_freem(m); + m_freem(m); goto out; } isipv6 = 0; #else /* !INET */ error = EAFNOSUPPORT; - if (m) - m_freem(m); + m_freem(m); goto out; #endif /* INET */ } else { if ((inp->inp_vflag & INP_IPV6) == 0) { - if (m) - m_freem(m); + m_freem(m); error = EAFNOSUPPORT; goto out; } @@ -1127,8 +1123,7 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, inp->inp_inc.inc_flags |= INC_ISIPV6; if ((error = prison_remote_ip6(td->td_ucred, &sin6->sin6_addr))) { - if (m) - m_freem(m); + m_freem(m); goto out; } isipv6 = 1; @@ -1137,23 +1132,11 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, } #endif /* INET6 */ default: - if (m) - m_freem(m); + m_freem(m); error = EAFNOSUPPORT; goto out; } } - if (control) { - /* TCP doesn't do control messages (rights, creds, etc) */ - if (control->m_len) { - m_freem(control); - if (m) - m_freem(m); - error = EINVAL; - goto out; - } - m_freem(control); /* empty control, just free it */ - } if (!(flags & PRUS_OOB)) { sbappendstream(&so->so_snd, m, flags); if (nam && tp->t_state < TCPS_SYN_SENT) { diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c index 16ae0a89bb15..62a07701df6c 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -1273,6 +1273,7 @@ udp_output(struct inpcb *inp, struct mbuf *m, struct sockaddr *addr, break; } m_freem(control); + control = NULL; } if (error) goto release; diff --git a/sys/netinet6/raw_ip6.c b/sys/netinet6/raw_ip6.c index 3d2af6e5c9e6..a369abb04bfc 100644 --- a/sys/netinet6/raw_ip6.c +++ b/sys/netinet6/raw_ip6.c @@ -867,7 +867,7 @@ rip6_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam, struct inpcb *inp; struct sockaddr_in6 tmp; struct sockaddr_in6 *dst; - int ret; + int error; inp = sotoinpcb(so); KASSERT(inp != NULL, ("rip6_send: inp == NULL")); @@ -876,8 +876,8 @@ rip6_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam, /* Unlocked read. */ if (so->so_state & SS_ISCONNECTED) { if (nam) { - m_freem(m); - return (EISCONN); + error = EISCONN; + goto release; } /* XXX */ bzero(&tmp, sizeof(tmp)); @@ -889,18 +889,15 @@ rip6_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam, INP_RUNLOCK(inp); dst = &tmp; } else { - if (nam == NULL) { - m_freem(m); - return (ENOTCONN); - } - if (nam->sa_family != AF_INET6) { - m_freem(m); - return (EAFNOSUPPORT); - } - if (nam->sa_len != sizeof(struct sockaddr_in6)) { - m_freem(m); - return (EINVAL); - } + error = 0; + if (nam == NULL) + error = ENOTCONN; + else if (nam->sa_family != AF_INET6) + error = EAFNOSUPPORT; + else if (nam->sa_len != sizeof(struct sockaddr_in6)) + error = EINVAL; + if (error != 0) + goto release; tmp = *(struct sockaddr_in6 *)nam; dst = &tmp; @@ -914,12 +911,17 @@ rip6_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam, "unspec. Assume AF_INET6\n"); dst->sin6_family = AF_INET6; } else if (dst->sin6_family != AF_INET6) { - m_freem(m); - return(EAFNOSUPPORT); + error = EAFNOSUPPORT; + goto release; } } - ret = rip6_output(m, so, dst, control); - return (ret); + return (rip6_output(m, so, dst, control)); + +release: + if (control != NULL) + m_freem(control); + m_freem(m); + return (error); } struct pr_usrreqs rip6_usrreqs = { diff --git a/sys/netinet6/sctp6_usrreq.c b/sys/netinet6/sctp6_usrreq.c index 1030fe1bbb68..3be7a3e25de8 100644 --- a/sys/netinet6/sctp6_usrreq.c +++ b/sys/netinet6/sctp6_usrreq.c @@ -713,6 +713,11 @@ sctp6_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr, #ifdef INET case AF_INET: if (addr->sa_len != sizeof(struct sockaddr_in)) { + if (control) { + SCTP_RELEASE_PKT(control); + control = NULL; + } + SCTP_RELEASE_PKT(m); SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL); return (EINVAL); } @@ -721,12 +726,22 @@ sctp6_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr, #ifdef INET6 case AF_INET6: if (addr->sa_len != sizeof(struct sockaddr_in6)) { + if (control) { + SCTP_RELEASE_PKT(control); + control = NULL; + } + SCTP_RELEASE_PKT(m); SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL); return (EINVAL); } break; #endif default: + if (control) { + SCTP_RELEASE_PKT(control); + control = NULL; + } + SCTP_RELEASE_PKT(m); SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL); return (EINVAL); } @@ -738,10 +753,20 @@ sctp6_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr, * v4 addr or v4-mapped addr */ if (addr->sa_family == AF_INET) { + if (control) { + SCTP_RELEASE_PKT(control); + control = NULL; + } + SCTP_RELEASE_PKT(m); SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL); return (EINVAL); } if (IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr)) { + if (control) { + SCTP_RELEASE_PKT(control); + control = NULL; + } + SCTP_RELEASE_PKT(m); SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL); return (EINVAL); } diff --git a/sys/netinet6/send.c b/sys/netinet6/send.c index 816f4d2b6541..0a2b3c65aab4 100644 --- a/sys/netinet6/send.c +++ b/sys/netinet6/send.c @@ -252,8 +252,11 @@ send_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam, m = NULL; err: + if (control != NULL) + m_freem(control); if (m != NULL) m_freem(m); + return (error); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202105260150.14Q1o66L042735>