git: d8acd2681bcf - main - Fix mbuf leaks in various pru_send implementations
Mark Johnston
markj at FreeBSD.org
Wed May 12 17:09:37 UTC 2021
The branch main has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=d8acd2681bcfb2ff7eb154df82f268b1cb191b4c
commit d8acd2681bcfb2ff7eb154df82f268b1cb191b4c
Author: Mark Johnston <markj at FreeBSD.org>
AuthorDate: 2021-05-12 13:39:36 +0000
Commit: Mark Johnston <markj at 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);
}
More information about the dev-commits-src-main
mailing list