Date: Wed, 12 May 2021 17:09:36 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: d8acd2681bcf - main - Fix mbuf leaks in various pru_send implementations Message-ID: <202105121709.14CH9ak4031794@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=d8acd2681bcfb2ff7eb154df82f268b1cb191b4c commit d8acd2681bcfb2ff7eb154df82f268b1cb191b4c Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2021-05-12 13:39:36 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2021-05-12 17:00:09 +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 MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D30151 --- 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 ae678136bade..ea86f66556ea 100644 --- a/sys/kern/uipc_socket.c +++ b/sys/kern/uipc_socket.c @@ -3741,7 +3741,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 061681ddc2bc..693cf553de44 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 aef2ee400c0e..677a83ab94cc 100644 --- a/sys/netinet6/send.c +++ b/sys/netinet6/send.c @@ -250,8 +250,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?202105121709.14CH9ak4031794>