Skip to content

Commit

Permalink
socket: Implement SO_RERROR
Browse files Browse the repository at this point in the history
SO_RERROR indicates that receive buffer overflows should be handled as
errors. Historically receive buffer overflows have been ignored and
programs could not tell if they missed messages or messages had been
truncated because of overflows. Since programs historically do not
expect to get receive overflow errors, this behavior is not the
default.

This is really really important for programs that use route(4) to keep
in sync with the system. If we loose a message then we need to reload
the full system state, otherwise the behaviour from that point is
undefined and can lead to chasing bogus bug reports.

Reviewed by:	philip (network), kbowling (transport), gbe (manpages)
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D26652

(cherry picked from commit 7045b16)
  • Loading branch information
rsmarples authored and kev009 committed Aug 11, 2021
1 parent 76aa36a commit f452713
Show file tree
Hide file tree
Showing 21 changed files with 100 additions and 35 deletions.
10 changes: 9 additions & 1 deletion lib/libc/sys/getsockopt.2
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
.\" @(#)getsockopt.2 8.4 (Berkeley) 5/2/95
.\" $FreeBSD$
.\"
.Dd June 3, 2020
.Dd February 8, 2021
.Dt GETSOCKOPT 2
.Os
.Sh NAME
Expand Down Expand Up @@ -177,6 +177,7 @@ for the socket
.It Dv SO_PROTOCOL Ta "get the protocol number for the socket (get only)"
.It Dv SO_PROTOTYPE Ta "SunOS alias for the Linux SO_PROTOCOL (get only)"
.It Dv SO_ERROR Ta "get and clear error on the socket (get only)"
.It Dv SO_RERROR Ta "enables receive error reporting"
.It Dv SO_SETFIB Ta "set the associated FIB (routing table) for the socket (set only)"
.El
.Pp
Expand Down Expand Up @@ -514,6 +515,13 @@ returns any pending error on the socket and clears
the error status.
It may be used to check for asynchronous errors on connected
datagram sockets or for other asynchronous errors.
.Dv SO_RERROR
indicates that receive buffer overflows should be handled as errors.
Historically receive buffer overflows have been ignored and programs
could not tell if they missed messages or messages had been truncated
because of overflows.
Since programs historically do not expect to get receive overflow errors,
this behavior is not the default.
.Pp
.Dv SO_LABEL
returns the MAC label of the socket.
Expand Down
13 changes: 12 additions & 1 deletion sbin/route/route.c
Original file line number Diff line number Diff line change
Expand Up @@ -1444,9 +1444,20 @@ monitor(int argc, char *argv[])
interfaces();
exit(0);
}

#ifdef SO_RERROR
n = 1;
if (setsockopt(s, SOL_SOCKET, SO_RERROR, &n, sizeof(n)) == -1)
warn("SO_RERROR");
#endif

for (;;) {
time_t now;
n = read(s, msg, 2048);
n = read(s, msg, sizeof(msg));
if (n == -1) {
warn("read");
continue;
}
now = time(NULL);
(void)printf("\ngot message of size %d on %s", n, ctime(&now));
print_rtmsg((struct rt_msghdr *)(void *)msg, n);
Expand Down
24 changes: 24 additions & 0 deletions sys/kern/uipc_sockbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,30 @@ socantrcvmore(struct socket *so)
mtx_assert(SOCKBUF_MTX(&so->so_rcv), MA_NOTOWNED);
}

void
soroverflow_locked(struct socket *so)
{

SOCKBUF_LOCK_ASSERT(&so->so_rcv);

if (so->so_options & SO_RERROR) {
so->so_rerror = ENOBUFS;
sorwakeup_locked(so);
} else
SOCKBUF_UNLOCK(&so->so_rcv);

mtx_assert(SOCKBUF_MTX(&so->so_rcv), MA_NOTOWNED);
}

void
soroverflow(struct socket *so)
{

SOCKBUF_LOCK(&so->so_rcv);
soroverflow_locked(so);
mtx_assert(SOCKBUF_MTX(&so->so_rcv), MA_NOTOWNED);
}

/*
* Wait for data to arrive at/drain from a socket buffer.
*/
Expand Down
30 changes: 22 additions & 8 deletions sys/kern/uipc_socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -1948,12 +1948,19 @@ soreceive_generic(struct socket *so, struct sockaddr **psa, struct uio *uio,
KASSERT(m != NULL || !sbavail(&so->so_rcv),
("receive: m == %p sbavail == %u",
m, sbavail(&so->so_rcv)));
if (so->so_error) {
if (so->so_error || so->so_rerror) {
if (m != NULL)
goto dontblock;
error = so->so_error;
if ((flags & MSG_PEEK) == 0)
so->so_error = 0;
if (so->so_error)
error = so->so_error;
else
error = so->so_rerror;
if ((flags & MSG_PEEK) == 0) {
if (so->so_error)
so->so_error = 0;
else
so->so_rerror = 0;
}
SOCKBUF_UNLOCK(&so->so_rcv);
goto release;
}
Expand Down Expand Up @@ -2297,7 +2304,7 @@ soreceive_generic(struct socket *so, struct sockaddr **psa, struct uio *uio,
while (flags & MSG_WAITALL && m == NULL && uio->uio_resid > 0 &&
!sosendallatonce(so) && nextrecord == NULL) {
SOCKBUF_LOCK_ASSERT(&so->so_rcv);
if (so->so_error ||
if (so->so_error || so->so_rerror ||
so->so_rcv.sb_state & SBS_CANTRCVMORE)
break;
/*
Expand Down Expand Up @@ -3038,6 +3045,7 @@ sosetopt(struct socket *so, struct sockopt *sopt)
case SO_NOSIGPIPE:
case SO_NO_DDP:
case SO_NO_OFFLOAD:
case SO_RERROR:
error = sooptcopyin(sopt, &optval, sizeof optval,
sizeof optval);
if (error)
Expand Down Expand Up @@ -3259,6 +3267,7 @@ sogetopt(struct socket *so, struct sockopt *sopt)
case SO_NOSIGPIPE:
case SO_NO_DDP:
case SO_NO_OFFLOAD:
case SO_RERROR:
optval = so->so_options & sopt->sopt_name;
integer:
error = sooptcopyout(sopt, &optval, sizeof optval);
Expand All @@ -3278,8 +3287,13 @@ sogetopt(struct socket *so, struct sockopt *sopt)

case SO_ERROR:
SOCK_LOCK(so);
optval = so->so_error;
so->so_error = 0;
if (so->so_error) {
optval = so->so_error;
so->so_error = 0;
} else {
optval = so->so_rerror;
so->so_rerror = 0;
}
SOCK_UNLOCK(so);
goto integer;

Expand Down Expand Up @@ -3832,7 +3846,7 @@ filt_soread(struct knote *kn, long hint)
kn->kn_flags |= EV_EOF;
kn->kn_fflags = so->so_error;
return (1);
} else if (so->so_error) /* temporary udp error */
} else if (so->so_error || so->so_rerror)
return (1);

if (kn->kn_sfflags & NOTE_LOWAT) {
Expand Down
2 changes: 1 addition & 1 deletion sys/kern/uipc_usrreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -1059,7 +1059,7 @@ uipc_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam,
m = NULL;
control = NULL;
} else {
SOCKBUF_UNLOCK(&so2->so_rcv);
soroverflow_locked(so2);
error = ENOBUFS;
}
if (nam != NULL)
Expand Down
11 changes: 6 additions & 5 deletions sys/net/raw_usrreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,20 +100,21 @@ raw_input_ext(struct mbuf *m0, struct sockproto *proto, struct sockaddr *src,
n = m_copym(m, 0, M_COPYALL, M_NOWAIT);
if (n) {
if (sbappendaddr(&last->so_rcv, src,
n, (struct mbuf *)0) == 0)
/* should notify about lost packet */
n, (struct mbuf *)0) == 0) {
soroverflow(last);
m_freem(n);
else
} else
sorwakeup(last);
}
}
last = rp->rcb_socket;
}
if (last) {
if (sbappendaddr(&last->so_rcv, src,
m, (struct mbuf *)0) == 0)
m, (struct mbuf *)0) == 0) {
soroverflow(last);
m_freem(m);
else
} else
sorwakeup(last);
} else
m_freem(m);
Expand Down
1 change: 1 addition & 0 deletions sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,7 @@ ng_btsocket_hci_raw_data_input(struct mbuf *nam)

NG_FREE_M(m);
NG_FREE_M(ctl);
soroverflow(pcb->so);
}
}
next:
Expand Down
2 changes: 1 addition & 1 deletion sys/netgraph/ng_socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ ngs_rcvmsg(node_p node, item_p item, hook_p lasthook)
/* Send it up to the socket. */
if (sbappendaddr_locked(&so->so_rcv, (struct sockaddr *)&addr, m,
NULL) == 0) {
SOCKBUF_UNLOCK(&so->so_rcv);
soroverflow_locked(so);
TRAP_ERROR;
m_freem(m);
return (ENOBUFS);
Expand Down
2 changes: 1 addition & 1 deletion sys/netinet/ip_divert.c
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ divert_packet(struct mbuf *m, bool incoming)
if (sbappendaddr_locked(&sa->so_rcv,
(struct sockaddr *)&divsrc, m,
(struct mbuf *)0) == 0) {
SOCKBUF_UNLOCK(&sa->so_rcv);
soroverflow_locked(sa);
sa = NULL; /* force mbuf reclaim below */
} else
sorwakeup_locked(sa);
Expand Down
2 changes: 1 addition & 1 deletion sys/netinet/ip_mroute.c
Original file line number Diff line number Diff line change
Expand Up @@ -1199,7 +1199,7 @@ socket_send(struct socket *s, struct mbuf *mm, struct sockaddr_in *src)
sorwakeup_locked(s);
return 0;
}
SOCKBUF_UNLOCK(&s->so_rcv);
soroverflow_locked(s);
}
m_freem(mm);
return -1;
Expand Down
3 changes: 1 addition & 2 deletions sys/netinet/raw_ip.c
Original file line number Diff line number Diff line change
Expand Up @@ -263,11 +263,10 @@ rip_append(struct inpcb *last, struct ip *ip, struct mbuf *n,
SOCKBUF_LOCK(&so->so_rcv);
if (sbappendaddr_locked(&so->so_rcv,
(struct sockaddr *)ripsrc, n, opts) == 0) {
/* should notify about lost packet */
soroverflow_locked(so);
m_freem(n);
if (opts)
m_freem(opts);
SOCKBUF_UNLOCK(&so->so_rcv);
} else
sorwakeup_locked(so);
} else
Expand Down
2 changes: 1 addition & 1 deletion sys/netinet/udp_usrreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ udp_append(struct inpcb *inp, struct ip *ip, struct mbuf *n, int off,
so = inp->inp_socket;
SOCKBUF_LOCK(&so->so_rcv);
if (sbappendaddr_locked(&so->so_rcv, append_sa, n, opts) == 0) {
SOCKBUF_UNLOCK(&so->so_rcv);
soroverflow(so);
m_freem(n);
if (opts)
m_freem(opts);
Expand Down
6 changes: 2 additions & 4 deletions sys/netinet6/icmp6.c
Original file line number Diff line number Diff line change
Expand Up @@ -1974,13 +1974,11 @@ icmp6_rip6_input(struct mbuf **mp, int off)
&last->inp_socket->so_rcv,
(struct sockaddr *)&fromsa, n, opts)
== 0) {
/* should notify about lost packet */
soroverflow_locked(last->inp_socket);
m_freem(n);
if (opts) {
m_freem(opts);
}
SOCKBUF_UNLOCK(
&last->inp_socket->so_rcv);
} else
sorwakeup_locked(last->inp_socket);
opts = NULL;
Expand Down Expand Up @@ -2020,7 +2018,7 @@ icmp6_rip6_input(struct mbuf **mp, int off)
m_freem(m);
if (opts)
m_freem(opts);
SOCKBUF_UNLOCK(&last->inp_socket->so_rcv);
soroverflow_locked(last->inp_socket);
} else
sorwakeup_locked(last->inp_socket);
INP_RUNLOCK(last);
Expand Down
1 change: 1 addition & 0 deletions sys/netinet6/ip6_input.c
Original file line number Diff line number Diff line change
Expand Up @@ -1575,6 +1575,7 @@ ip6_notify_pmtu(struct inpcb *inp, struct sockaddr_in6 *dst, u_int32_t mtu)
so = inp->inp_socket;
if (sbappendaddr(&so->so_rcv, (struct sockaddr *)dst, NULL, m_mtu)
== 0) {
soroverflow(so);
m_freem(m_mtu);
/* XXX: should count statistics */
} else
Expand Down
3 changes: 2 additions & 1 deletion sys/netinet6/ip6_mroute.c
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,8 @@ socket_send(struct socket *s, struct mbuf *mm, struct sockaddr_in6 *src)
mm, (struct mbuf *)0) != 0) {
sorwakeup(s);
return (0);
}
} else
soroverflow(s);
}
m_freem(mm);
return (-1);
Expand Down
2 changes: 2 additions & 0 deletions sys/netinet6/raw_ip6.c
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ rip6_input(struct mbuf **mp, int *offp, int proto)
if (sbappendaddr(&last->inp_socket->so_rcv,
(struct sockaddr *)&fromsa,
n, opts) == 0) {
soroverflow(last->inp_socket);
m_freem(n);
if (opts)
m_freem(opts);
Expand Down Expand Up @@ -325,6 +326,7 @@ rip6_input(struct mbuf **mp, int *offp, int proto)
m_adj(m, *offp);
if (sbappendaddr(&last->inp_socket->so_rcv,
(struct sockaddr *)&fromsa, m, opts) == 0) {
soroverflow(last->inp_socket);
m_freem(m);
if (opts)
m_freem(opts);
Expand Down
2 changes: 1 addition & 1 deletion sys/netinet6/send.c
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ send_input(struct mbuf *m, struct ifnet *ifp, int direction, int msglen __unused
SOCKBUF_LOCK(&V_send_so->so_rcv);
if (sbappendaddr_locked(&V_send_so->so_rcv,
(struct sockaddr *)&sendsrc, m, NULL) == 0) {
SOCKBUF_UNLOCK(&V_send_so->so_rcv);
soroverflow_locked(V_send_so);
/* XXX stats. */
m_freem(m);
} else {
Expand Down
2 changes: 1 addition & 1 deletion sys/netinet6/udp6_usrreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ udp6_append(struct inpcb *inp, struct mbuf *n, int off,
SOCKBUF_LOCK(&so->so_rcv);
if (sbappendaddr_locked(&so->so_rcv, (struct sockaddr *)&fromsa[0], n,
opts) == 0) {
SOCKBUF_UNLOCK(&so->so_rcv);
soroverflow_locked(so);
m_freem(n);
if (opts)
m_freem(opts);
Expand Down
10 changes: 5 additions & 5 deletions sys/netipsec/keysock.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ key_output(struct mbuf *m, struct socket *so, ...)
static int
key_sendup0(struct rawcb *rp, struct mbuf *m, int promisc)
{
int error;

if (promisc) {
struct sadb_msg *pmsg;
Expand All @@ -165,11 +164,12 @@ key_sendup0(struct rawcb *rp, struct mbuf *m, int promisc)
m, NULL)) {
PFKEYSTAT_INC(in_nomem);
m_freem(m);
error = ENOBUFS;
} else
error = 0;
soroverflow(rp->rcb_socket);
return ENOBUFS;
}

sorwakeup(rp->rcb_socket);
return error;
return 0;
}

/* so can be NULL if target != KEY_SENDUP_ONE */
Expand Down
1 change: 1 addition & 0 deletions sys/sys/socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ typedef __uintptr_t uintptr_t;
#define SO_NO_OFFLOAD 0x00004000 /* socket cannot be offloaded */
#define SO_NO_DDP 0x00008000 /* disable direct data placement */
#define SO_REUSEPORT_LB 0x00010000 /* reuse with load balancing */
#define SO_RERROR 0x00020000 /* keep track of receive errors */

/*
* Additional options, not kept in so_options.
Expand Down
6 changes: 5 additions & 1 deletion sys/sys/socketvar.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ struct socket {
struct protosw *so_proto; /* (a) protocol handle */
short so_timeo; /* (g) connection timeout */
u_short so_error; /* (f) error affecting connection */
u_short so_rerror; /* (f) error affecting connection */
struct sigio *so_sigio; /* [sg] information for async I/O or
out of band data (SIGURG) */
struct ucred *so_cred; /* (a) user credentials */
Expand Down Expand Up @@ -266,7 +267,8 @@ struct socket {

/* can we read something from so? */
#define soreadabledata(so) \
(sbavail(&(so)->so_rcv) >= (so)->so_rcv.sb_lowat || (so)->so_error)
(sbavail(&(so)->so_rcv) >= (so)->so_rcv.sb_lowat || \
(so)->so_error || (so)->so_rerror)
#define soreadable(so) \
(soreadabledata(so) || ((so)->so_rcv.sb_state & SBS_CANTRCVMORE))

Expand Down Expand Up @@ -480,6 +482,8 @@ void socantrcvmore(struct socket *so);
void socantrcvmore_locked(struct socket *so);
void socantsendmore(struct socket *so);
void socantsendmore_locked(struct socket *so);
void soroverflow(struct socket *so);
void soroverflow_locked(struct socket *so);

/*
* Accept filter functions (duh).
Expand Down

0 comments on commit f452713

Please sign in to comment.