Date: Wed, 11 Aug 2021 01:57:55 GMT From: Kevin Bowling <kbowling@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: f45271340836 - stable/13 - socket: Implement SO_RERROR Message-ID: <202108110157.17B1vtEL079990@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by kbowling (ports committer): URL: https://cgit.FreeBSD.org/src/commit/?id=f45271340836769d53320d6dbcf63226bb4e39e3 commit f45271340836769d53320d6dbcf63226bb4e39e3 Author: Roy Marples <roy@marples.name> AuthorDate: 2021-07-28 15:46:59 +0000 Commit: Kevin Bowling <kbowling@FreeBSD.org> CommitDate: 2021-08-11 01:54:00 +0000 socket: Implement SO_RERROR 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 7045b1603bdf054145dd958a4acc17b410fb62a0) --- lib/libc/sys/getsockopt.2 | 10 +++++++- sbin/route/route.c | 13 +++++++++- sys/kern/uipc_sockbuf.c | 24 +++++++++++++++++ sys/kern/uipc_socket.c | 30 ++++++++++++++++------ sys/kern/uipc_usrreq.c | 2 +- sys/net/raw_usrreq.c | 11 ++++---- .../bluetooth/socket/ng_btsocket_hci_raw.c | 1 + sys/netgraph/ng_socket.c | 2 +- sys/netinet/ip_divert.c | 2 +- sys/netinet/ip_mroute.c | 2 +- sys/netinet/raw_ip.c | 3 +-- sys/netinet/udp_usrreq.c | 2 +- sys/netinet6/icmp6.c | 6 ++--- sys/netinet6/ip6_input.c | 1 + sys/netinet6/ip6_mroute.c | 3 ++- sys/netinet6/raw_ip6.c | 2 ++ sys/netinet6/send.c | 2 +- sys/netinet6/udp6_usrreq.c | 2 +- sys/netipsec/keysock.c | 10 ++++---- sys/sys/socket.h | 1 + sys/sys/socketvar.h | 6 ++++- 21 files changed, 100 insertions(+), 35 deletions(-) diff --git a/lib/libc/sys/getsockopt.2 b/lib/libc/sys/getsockopt.2 index 1e4ed6827170..3ff971a0e5db 100644 --- a/lib/libc/sys/getsockopt.2 +++ b/lib/libc/sys/getsockopt.2 @@ -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 @@ -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 @@ -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. diff --git a/sbin/route/route.c b/sbin/route/route.c index 51a0c68746a6..b16fb6d17a08 100644 --- a/sbin/route/route.c +++ b/sbin/route/route.c @@ -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); diff --git a/sys/kern/uipc_sockbuf.c b/sys/kern/uipc_sockbuf.c index 2c0e10ee1dc2..b2202fe15192 100644 --- a/sys/kern/uipc_sockbuf.c +++ b/sys/kern/uipc_sockbuf.c @@ -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. */ diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c index 9f8ad921933b..9f4e2fc64e98 100644 --- a/sys/kern/uipc_socket.c +++ b/sys/kern/uipc_socket.c @@ -1948,12 +1948,19 @@ restart: 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; } @@ -2297,7 +2304,7 @@ dontblock: 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; /* @@ -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) @@ -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); @@ -3278,8 +3287,13 @@ integer: 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; @@ -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) { diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c index a01171424320..3d7daac42001 100644 --- a/sys/kern/uipc_usrreq.c +++ b/sys/kern/uipc_usrreq.c @@ -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) diff --git a/sys/net/raw_usrreq.c b/sys/net/raw_usrreq.c index f43de7dae9ca..5d4e223e5a0a 100644 --- a/sys/net/raw_usrreq.c +++ b/sys/net/raw_usrreq.c @@ -100,10 +100,10 @@ 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); } } @@ -111,9 +111,10 @@ raw_input_ext(struct mbuf *m0, struct sockproto *proto, struct sockaddr *src, } 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); diff --git a/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c b/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c index 5f6b98d03359..c82515f82631 100644 --- a/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c +++ b/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c @@ -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: diff --git a/sys/netgraph/ng_socket.c b/sys/netgraph/ng_socket.c index 1c67099a4dc4..05f37579aba1 100644 --- a/sys/netgraph/ng_socket.c +++ b/sys/netgraph/ng_socket.c @@ -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); diff --git a/sys/netinet/ip_divert.c b/sys/netinet/ip_divert.c index 77a4bfcd08ac..4141386a6a2d 100644 --- a/sys/netinet/ip_divert.c +++ b/sys/netinet/ip_divert.c @@ -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); diff --git a/sys/netinet/ip_mroute.c b/sys/netinet/ip_mroute.c index b66fe8df0793..6aeea44f631c 100644 --- a/sys/netinet/ip_mroute.c +++ b/sys/netinet/ip_mroute.c @@ -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; diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c index 996440227145..afbdc14e47c4 100644 --- a/sys/netinet/raw_ip.c +++ b/sys/netinet/raw_ip.c @@ -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 diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c index 5c9dbd36a1d6..ed79ddce5109 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -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); diff --git a/sys/netinet6/icmp6.c b/sys/netinet6/icmp6.c index e17f82a54951..6b8f0f7be5bb 100644 --- a/sys/netinet6/icmp6.c +++ b/sys/netinet6/icmp6.c @@ -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; @@ -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); diff --git a/sys/netinet6/ip6_input.c b/sys/netinet6/ip6_input.c index 9ea578f88417..30ad9a53006a 100644 --- a/sys/netinet6/ip6_input.c +++ b/sys/netinet6/ip6_input.c @@ -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 diff --git a/sys/netinet6/ip6_mroute.c b/sys/netinet6/ip6_mroute.c index d2277e41110c..503c83490940 100644 --- a/sys/netinet6/ip6_mroute.c +++ b/sys/netinet6/ip6_mroute.c @@ -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); diff --git a/sys/netinet6/raw_ip6.c b/sys/netinet6/raw_ip6.c index a369abb04bfc..3e3df65a0e12 100644 --- a/sys/netinet6/raw_ip6.c +++ b/sys/netinet6/raw_ip6.c @@ -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); @@ -325,6 +326,7 @@ skip_2: 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); diff --git a/sys/netinet6/send.c b/sys/netinet6/send.c index 0a2b3c65aab4..a69afd7b5ed6 100644 --- a/sys/netinet6/send.c +++ b/sys/netinet6/send.c @@ -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 { diff --git a/sys/netinet6/udp6_usrreq.c b/sys/netinet6/udp6_usrreq.c index 7c573d095d77..816e3bdd2850 100644 --- a/sys/netinet6/udp6_usrreq.c +++ b/sys/netinet6/udp6_usrreq.c @@ -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); diff --git a/sys/netipsec/keysock.c b/sys/netipsec/keysock.c index 317eb53289cf..aa893b131a57 100644 --- a/sys/netipsec/keysock.c +++ b/sys/netipsec/keysock.c @@ -141,7 +141,6 @@ end: static int key_sendup0(struct rawcb *rp, struct mbuf *m, int promisc) { - int error; if (promisc) { struct sadb_msg *pmsg; @@ -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 */ diff --git a/sys/sys/socket.h b/sys/sys/socket.h index d9256fd7544a..2cb76f9c6d63 100644 --- a/sys/sys/socket.h +++ b/sys/sys/socket.h @@ -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. diff --git a/sys/sys/socketvar.h b/sys/sys/socketvar.h index dbd9804a980d..47033fdabbfa 100644 --- a/sys/sys/socketvar.h +++ b/sys/sys/socketvar.h @@ -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 */ @@ -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)) @@ -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).
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202108110157.17B1vtEL079990>