Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 8 Feb 2021 22:32:44 GMT
From:      "Alexander V. Chernikov" <melifaro@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 924d1c9a050d - main - Revert "SO_RERROR indicates that receive buffer overflows should be handled as errors." Wrong version of the change was pushed inadvertenly.
Message-ID:  <202102082232.118MWieO029527@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by melifaro:

URL: https://cgit.FreeBSD.org/src/commit/?id=924d1c9a050d7e355d0562fca62bd2bb9b5f53d7

commit 924d1c9a050d7e355d0562fca62bd2bb9b5f53d7
Author:     Alexander V. Chernikov <melifaro@FreeBSD.org>
AuthorDate: 2021-02-08 22:30:39 +0000
Commit:     Alexander V. Chernikov <melifaro@FreeBSD.org>
CommitDate: 2021-02-08 22:32:32 +0000

    Revert "SO_RERROR indicates that receive buffer overflows should be handled as errors."
    Wrong version of the change was pushed inadvertenly.
    
    This reverts commit 4a01b854ca5c2e5124958363b3326708b913af71.
---
 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, 35 insertions(+), 100 deletions(-)

diff --git a/lib/libc/sys/getsockopt.2 b/lib/libc/sys/getsockopt.2
index 223eb3e8a28b..1e4ed6827170 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 Feb 07, 2020
+.Dd June 3, 2020
 .Dt GETSOCKOPT 2
 .Os
 .Sh NAME
@@ -177,7 +177,6 @@ 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
@@ -515,13 +514,6 @@ 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 b16fb6d17a08..51a0c68746a6 100644
--- a/sbin/route/route.c
+++ b/sbin/route/route.c
@@ -1444,20 +1444,9 @@ 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, sizeof(msg));
-		if (n == -1) {
-			warn("read");
-			continue;
-		}
+		n = read(s, msg, 2048);
 		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 b5ecdb99b59b..cf53f234d8fc 100644
--- a/sys/kern/uipc_sockbuf.c
+++ b/sys/kern/uipc_sockbuf.c
@@ -436,30 +436,6 @@ 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 8d218cf10a81..7b16401b7071 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1953,19 +1953,12 @@ restart:
 		KASSERT(m != NULL || !sbavail(&so->so_rcv),
 		    ("receive: m == %p sbavail == %u",
 		    m, sbavail(&so->so_rcv)));
-		if (so->so_error || so->so_rerror) {
+		if (so->so_error) {
 			if (m != NULL)
 				goto dontblock;
-			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;
-			}
+			error = so->so_error;
+			if ((flags & MSG_PEEK) == 0)
+				so->so_error = 0;
 			SOCKBUF_UNLOCK(&so->so_rcv);
 			goto release;
 		}
@@ -2309,7 +2302,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 || so->so_rerror ||
+			if (so->so_error ||
 			    so->so_rcv.sb_state & SBS_CANTRCVMORE)
 				break;
 			/*
@@ -3050,7 +3043,6 @@ 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)
@@ -3272,7 +3264,6 @@ 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);
@@ -3292,13 +3283,8 @@ integer:
 
 		case SO_ERROR:
 			SOCK_LOCK(so);
-			if (so->so_error) {
-				optval = so->so_error;
-				so->so_error = 0;
-			} else {
-				optval = so->so_rerror;
-				so->so_rerror = 0;
-			}
+			optval = so->so_error;
+			so->so_error = 0;
 			SOCK_UNLOCK(so);
 			goto integer;
 
@@ -3847,7 +3833,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 || so->so_rerror)
+	} else if (so->so_error)	/* temporary udp error */
 		return (1);
 
 	if (kn->kn_sfflags & NOTE_LOWAT) {
diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
index 10680ad88136..0809f5180cc1 100644
--- a/sys/kern/uipc_usrreq.c
+++ b/sys/kern/uipc_usrreq.c
@@ -1058,7 +1058,7 @@ uipc_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam,
 			m = NULL;
 			control = NULL;
 		} else {
-			soroverflow_locked(so2);
+			SOCKBUF_UNLOCK(&so2->so_rcv);
 			error = ENOBUFS;
 		}
 		if (nam != NULL)
diff --git a/sys/net/raw_usrreq.c b/sys/net/raw_usrreq.c
index 5d4e223e5a0a..f43de7dae9ca 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) {
-					soroverflow(last);
+				    n, (struct mbuf *)0) == 0)
+					/* should notify about lost packet */
 					m_freem(n);
-				} else
+				else
 					sorwakeup(last);
 			}
 		}
@@ -111,10 +111,9 @@ 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) {
-			soroverflow(last);
+		    m, (struct mbuf *)0) == 0)
 			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 c82515f82631..5f6b98d03359 100644
--- a/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c
+++ b/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c
@@ -539,7 +539,6 @@ 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 905567c86b47..865e9dd7948f 100644
--- a/sys/netgraph/ng_socket.c
+++ b/sys/netgraph/ng_socket.c
@@ -982,7 +982,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) {
-		soroverflow_locked(so);
+		SOCKBUF_UNLOCK(&so->so_rcv);
 		TRAP_ERROR;
 		m_freem(m);
 		return (ENOBUFS);
diff --git a/sys/netinet/ip_divert.c b/sys/netinet/ip_divert.c
index 99a8405fe334..65f1d263b5fa 100644
--- a/sys/netinet/ip_divert.c
+++ b/sys/netinet/ip_divert.c
@@ -285,7 +285,7 @@ divert_packet(struct mbuf *m, bool incoming)
 			if (sbappendaddr_locked(&sa->so_rcv,
 			    (struct sockaddr *)&divsrc, m,
 			    (struct mbuf *)0) == 0) {
-				soroverflow_locked(sa);
+				SOCKBUF_UNLOCK(&sa->so_rcv);
 				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 6aeea44f631c..b66fe8df0793 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;
 	}
-	soroverflow_locked(s);
+	SOCKBUF_UNLOCK(&s->so_rcv);
     }
     m_freem(mm);
     return -1;
diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c
index 1df54a8cfdba..c9def015343c 100644
--- a/sys/netinet/raw_ip.c
+++ b/sys/netinet/raw_ip.c
@@ -263,10 +263,11 @@ 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) {
-			soroverflow_locked(so);
+			/* should notify about lost packet */
 			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 6392c6c441d6..52304ddd6584 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) {
-		soroverflow(so);
+		SOCKBUF_UNLOCK(&so->so_rcv);
 		m_freem(n);
 		if (opts)
 			m_freem(opts);
diff --git a/sys/netinet6/icmp6.c b/sys/netinet6/icmp6.c
index 71007d5e9d11..1a08dad7de64 100644
--- a/sys/netinet6/icmp6.c
+++ b/sys/netinet6/icmp6.c
@@ -1973,11 +1973,13 @@ icmp6_rip6_input(struct mbuf **mp, int off)
 				    &last->inp_socket->so_rcv,
 				    (struct sockaddr *)&fromsa, n, opts)
 				    == 0) {
-					soroverflow_locked(last->inp_socket);
+					/* should notify about lost packet */
 					m_freem(n);
 					if (opts) {
 						m_freem(opts);
 					}
+					SOCKBUF_UNLOCK(
+					    &last->inp_socket->so_rcv);
 				} else
 					sorwakeup_locked(last->inp_socket);
 				opts = NULL;
@@ -2017,7 +2019,7 @@ icmp6_rip6_input(struct mbuf **mp, int off)
 			m_freem(m);
 			if (opts)
 				m_freem(opts);
-			soroverflow_locked(last->inp_socket);
+			SOCKBUF_UNLOCK(&last->inp_socket->so_rcv);
 		} else
 			sorwakeup_locked(last->inp_socket);
 		INP_RUNLOCK(last);
diff --git a/sys/netinet6/ip6_input.c b/sys/netinet6/ip6_input.c
index 9a4fd20a52b8..8f500cb87bfe 100644
--- a/sys/netinet6/ip6_input.c
+++ b/sys/netinet6/ip6_input.c
@@ -1576,7 +1576,6 @@ 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 503c83490940..d2277e41110c 100644
--- a/sys/netinet6/ip6_mroute.c
+++ b/sys/netinet6/ip6_mroute.c
@@ -1038,8 +1038,7 @@ 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 92fe8b7e2253..aea99add4391 100644
--- a/sys/netinet6/raw_ip6.c
+++ b/sys/netinet6/raw_ip6.c
@@ -214,7 +214,6 @@ 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);
@@ -326,7 +325,6 @@ 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 8458ef367cba..bc9880c82267 100644
--- a/sys/netinet6/send.c
+++ b/sys/netinet6/send.c
@@ -291,7 +291,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) {
-		soroverflow_locked(V_send_so);
+		SOCKBUF_UNLOCK(&V_send_so->so_rcv);
 		/* XXX stats. */
 		m_freem(m);
 	} else {
diff --git a/sys/netinet6/udp6_usrreq.c b/sys/netinet6/udp6_usrreq.c
index bc1d8cc5b2fa..1535be90e1b0 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) {
-		soroverflow_locked(so);
+		SOCKBUF_UNLOCK(&so->so_rcv);
 		m_freem(n);
 		if (opts)
 			m_freem(opts);
diff --git a/sys/netipsec/keysock.c b/sys/netipsec/keysock.c
index 7a0b9e757022..49efa0a3c510 100644
--- a/sys/netipsec/keysock.c
+++ b/sys/netipsec/keysock.c
@@ -141,6 +141,7 @@ end:
 static int
 key_sendup0(struct rawcb *rp, struct mbuf *m, int promisc)
 {
+	int error;
 
 	if (promisc) {
 		struct sadb_msg *pmsg;
@@ -164,12 +165,11 @@ key_sendup0(struct rawcb *rp, struct mbuf *m, int promisc)
 	    m, NULL)) {
 		PFKEYSTAT_INC(in_nomem);
 		m_freem(m);
-		soroverflow(rp->rcb_socket);
-		return ENOBUFS;
-	}
-
+		error = ENOBUFS;
+	} else
+		error = 0;
 	sorwakeup(rp->rcb_socket);
-	return 0;
+	return error;
 }
 
 /* so can be NULL if target != KEY_SENDUP_ONE */
diff --git a/sys/sys/socket.h b/sys/sys/socket.h
index 2cb76f9c6d63..d9256fd7544a 100644
--- a/sys/sys/socket.h
+++ b/sys/sys/socket.h
@@ -147,7 +147,6 @@ 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 60a4dde7f0cb..295a1cf3d37f 100644
--- a/sys/sys/socketvar.h
+++ b/sys/sys/socketvar.h
@@ -100,7 +100,6 @@ 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 */
@@ -267,8 +266,7 @@ struct socket {
 
 /* can we read something from so? */
 #define	soreadabledata(so) \
-	(sbavail(&(so)->so_rcv) >= (so)->so_rcv.sb_lowat || \
-	(so)->so_error || (so)->so_rerror)
+	(sbavail(&(so)->so_rcv) >= (so)->so_rcv.sb_lowat ||  (so)->so_error)
 #define	soreadable(so) \
 	(soreadabledata(so) || ((so)->so_rcv.sb_state & SBS_CANTRCVMORE))
 
@@ -482,8 +480,6 @@ 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?202102082232.118MWieO029527>