tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Detecting buffer overflows



On 07/03/2018 21:53, Roy Marples wrote:
Attached is a patch that introduces the kevent(2) NOTE_OVERFLOW which is set when the socket receives buffer overflows.

ifwatchd and dhcpcd can then spot the overflow, flush the buffer (like closing and re-opening the socket), re-examine the current state of affairs and do the right thing.
wpa_supplicant can't do much other than just log that it happened.

Commentary on this welcome, especially the kqueue specific parts.

New patch for this, changes:
Socket state SS_RECVOVERFLOW introduced on overflow and is cleared when the socket is read from. NOTE_OVERFLOW is only set when asked for - this is because it needs to be removed from fflags when EV_EOF is set in flags. NOTE_OVERFLOW now uses bits that errno shouldn't touch.
To clear NOTE_OVERFLOW, EV_CLEAR should be used.
man page updates for the patch.

Commentary welcome.

Roy
Index: lib/libc/sys/kqueue.2
===================================================================
RCS file: /cvsroot/src/lib/libc/sys/kqueue.2,v
retrieving revision 1.47
diff -u -p -r1.47 kqueue.2
--- lib/libc/sys/kqueue.2	9 Jan 2018 03:31:12 -0000	1.47
+++ lib/libc/sys/kqueue.2	15 Mar 2018 16:34:13 -0000
@@ -32,7 +32,7 @@
 .\"
 .\" $FreeBSD: src/lib/libc/sys/kqueue.2,v 1.22 2001/06/27 19:55:57 dd Exp $
 .\"
-.Dd January 8, 2018
+.Dd March 15, 2018
 .Dt KQUEUE 2
 .Os
 .Sh NAME
@@ -334,6 +334,17 @@ and returns the socket error (if any) in
 .Va fflags .
 It is possible for EOF to be returned (indicating the connection is gone)
 while there is still data pending in the socket buffer.
+.Pp
+If NOTE_OVERFLOW was initially set in
+.Va fflags ,
+then it will also be set in
+.Va fflags
+on return when the socket buffer overflows and data has been lost.
+If
+.Va flags
+was set to EV_EOF then NOTE_OVERFLOW needs to be removed from
+.Va fflags
+to get the real error.
 .It Vnodes
 Returns when the file pointer is not at the end of file.
 .Va data
Index: share/man/man4/route.4
===================================================================
RCS file: /cvsroot/src/share/man/man4/route.4,v
retrieving revision 1.31
diff -u -p -r1.31 route.4
--- share/man/man4/route.4	3 Jul 2017 21:30:58 -0000	1.31
+++ share/man/man4/route.4	15 Mar 2018 16:34:13 -0000
@@ -29,7 +29,7 @@
 .\"
 .\"     @(#)route.4	8.6 (Berkeley) 4/19/94
 .\"
-.Dd April 11, 2017
+.Dd March 15, 2018
 .Dt ROUTE 4
 .Os
 .Sh NAME
@@ -371,7 +371,12 @@ Flags for IPv6 addresses:
 #define IN6_IFF_AUTOCONF	0x40	/* autoconfigurable address. */
 #define IN6_IFF_TEMPORARY	0x80	/* temporary (anonymous) address. */
 .Ed
+.Sh NOTES
+It's possible for the route socket buffer to overflow.
+.Xr kqueue 2
+sets NOTE_OVERFLOW when this happens.
 .Sh SEE ALSO
+.Xr kqueue 2 ,
 .Xr socket 2 ,
 .Xr sysctl 3
 .Sh HISTORY
Index: sys/kern/uipc_socket.c
===================================================================
RCS file: /cvsroot/src/sys/kern/uipc_socket.c,v
retrieving revision 1.259
diff -u -p -r1.259 uipc_socket.c
--- sys/kern/uipc_socket.c	4 Jan 2018 01:42:25 -0000	1.259
+++ sys/kern/uipc_socket.c	15 Mar 2018 16:34:13 -0000
@@ -1428,6 +1428,8 @@ soreceive(struct socket *so, struct mbuf
 	/* If m is non-NULL, we have some data to read. */
 	if (__predict_true(m != NULL)) {
 		type = m->m_type;
+		if ((flags & MSG_PEEK) == 0)
+			so->so_state &= ~SS_RECVOVERFLOW;
 		if (type == MT_OOBDATA)
 			flags |= MSG_OOB;
 	}
@@ -2248,8 +2250,10 @@ filt_soread(struct knote *kn, long hint)
 		solock(so);
 	kn->kn_data = so->so_rcv.sb_cc;
 	if (so->so_state & SS_CANTRCVMORE) {
+		int oflags = kn->kn_flags & NOTE_OVERFLOW;
+
 		kn->kn_flags |= EV_EOF;
-		kn->kn_fflags = so->so_error;
+		kn->kn_fflags = so->so_error | oflags;
 		rv = 1;
 	} else if (so->so_error)	/* temporary udp error */
 		rv = 1;
@@ -2257,6 +2261,8 @@ filt_soread(struct knote *kn, long hint)
 		rv = (kn->kn_data >= kn->kn_sdata);
 	else
 		rv = (kn->kn_data >= so->so_rcv.sb_lowat);
+	if (kn->kn_sfflags & NOTE_OVERFLOW && so->so_state & SS_RECVOVERFLOW)
+		kn->kn_fflags |= NOTE_OVERFLOW;
 	if (hint != NOTE_SUBMIT)
 		sounlock(so);
 	return rv;
Index: sys/kern/uipc_socket2.c
===================================================================
RCS file: /cvsroot/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.126
diff -u -p -r1.126 uipc_socket2.c
--- sys/kern/uipc_socket2.c	6 Jul 2017 17:42:39 -0000	1.126
+++ sys/kern/uipc_socket2.c	15 Mar 2018 16:34:13 -0000
@@ -495,6 +495,20 @@ socantrcvmore(struct socket *so)
 }
 
 /*
+ * soroverflow(): indicates that data was attempted to be sent
+ * but the receiving buffer overflowed.
+ */
+void
+soroverflow(struct socket *so)
+{
+	KASSERT(solocked(so));
+
+	so->so_rcv.sb_overflowed++;
+	so->so_state |= SS_RECVOVERFLOW;
+	sorwakeup(so);
+}
+
+/*
  * Wait for data to arrive at/drain from a socket buffer.
  */
 int
Index: sys/kern/uipc_usrreq.c
===================================================================
RCS file: /cvsroot/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.183
diff -u -p -r1.183 uipc_usrreq.c
--- sys/kern/uipc_usrreq.c	17 Feb 2018 20:19:36 -0000	1.183
+++ sys/kern/uipc_usrreq.c	15 Mar 2018 16:34:13 -0000
@@ -342,10 +342,10 @@ unp_output(struct mbuf *m, struct mbuf *
 #endif
 	if (sbappendaddr(&so2->so_rcv, (const struct sockaddr *)sun, m,
 	    control) == 0) {
-		so2->so_rcv.sb_overflowed++;
 		unp_dispose(control);
 		m_freem(control);
 		m_freem(m);
+		soroverflow(so2);
 		return (ENOBUFS);
 	} else {
 		sorwakeup(so2);
Index: sys/net/raw_usrreq.c
===================================================================
RCS file: /cvsroot/src/sys/net/raw_usrreq.c,v
retrieving revision 1.58
diff -u -p -r1.58 raw_usrreq.c
--- sys/net/raw_usrreq.c	25 Sep 2017 01:57:54 -0000	1.58
+++ sys/net/raw_usrreq.c	15 Mar 2018 16:34:13 -0000
@@ -106,21 +106,26 @@ raw_input(struct mbuf *m0, ...)
 			continue;
 		if (last != NULL) {
 			struct mbuf *n;
-			if ((n = m_copy(m, 0, M_COPYALL)) == NULL)
-				;
-			else if (sbappendaddr(&last->so_rcv, src, n, NULL) == 0)
-				/* should notify about lost packet */
-				m_freem(n);
-			else {
+
+			if ((n = m_copy(m, 0, M_COPYALL)) == NULL ||
+			    sbappendaddr(&last->so_rcv, src, n, NULL) == 0)
+			{
+				if (n != NULL)
+					m_freem(n);
+				soroverflow(last);
+			} else
 				sorwakeup(last);
-			}
 		}
 		last = rp->rcb_socket;
 	}
-	if (last == NULL || sbappendaddr(&last->so_rcv, src, m, NULL) == 0)
-		m_freem(m);
-	else {
-		sorwakeup(last);
+	if (last != NULL) {
+		if (sbappendaddr(&last->so_rcv, src, m, NULL) == 0) {
+			m_free(m);
+			soroverflow(last);
+		} else
+			sorwakeup(last);
+	} else {
+		m_free(m);
 	}
 }
 
Index: sys/net/rtsock.c
===================================================================
RCS file: /cvsroot/src/sys/net/rtsock.c,v
retrieving revision 1.238
diff -u -p -r1.238 rtsock.c
--- sys/net/rtsock.c	25 Jan 2018 03:09:05 -0000	1.238
+++ sys/net/rtsock.c	15 Mar 2018 16:34:13 -0000
@@ -2114,6 +2114,7 @@ COMPATNAME(route_enqueue)(struct mbuf *m
 
 	IFQ_LOCK(&ri->ri_intrq);
 	if (IF_QFULL(&ri->ri_intrq)) {
+		printf("%s: queue full, dropped message\n", __func__);
 		IF_DROP(&ri->ri_intrq);
 		IFQ_UNLOCK(&ri->ri_intrq);
 		m_freem(m);
Index: sys/netinet/udp_usrreq.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.245
diff -u -p -r1.245 udp_usrreq.c
--- sys/netinet/udp_usrreq.c	28 Feb 2018 11:23:24 -0000	1.245
+++ sys/netinet/udp_usrreq.c	15 Mar 2018 16:34:13 -0000
@@ -498,8 +498,8 @@ udp4_sendup(struct mbuf *m, int off /* o
 			m_freem(n);
 			if (opts)
 				m_freem(opts);
-			so->so_rcv.sb_overflowed++;
 			UDP_STATINC(UDP_STAT_FULLSOCK);
+			soroverflow(so);
 		} else
 			sorwakeup(so);
 	}
Index: sys/netinet6/udp6_usrreq.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/udp6_usrreq.c,v
retrieving revision 1.137
diff -u -p -r1.137 udp6_usrreq.c
--- sys/netinet6/udp6_usrreq.c	28 Feb 2018 11:23:24 -0000	1.137
+++ sys/netinet6/udp6_usrreq.c	15 Mar 2018 16:34:13 -0000
@@ -372,8 +372,8 @@ udp6_sendup(struct mbuf *m, int off /* o
 			m_freem(n);
 			if (opts)
 				m_freem(opts);
-			so->so_rcv.sb_overflowed++;
 			UDP6_STATINC(UDP6_STAT_FULLSOCK);
+			soroverflow(so);
 		} else
 			sorwakeup(so);
 	}
Index: sys/netipsec/keysock.c
===================================================================
RCS file: /cvsroot/src/sys/netipsec/keysock.c,v
retrieving revision 1.62
diff -u -p -r1.62 keysock.c
--- sys/netipsec/keysock.c	28 Sep 2017 17:21:42 -0000	1.62
+++ sys/netipsec/keysock.c	15 Mar 2018 16:34:13 -0000
@@ -207,11 +207,12 @@ key_sendup0(
 		    __func__);
 		PFKEY_STATINC(PFKEY_STAT_IN_NOMEM);
 		m_freem(m);
+		soroverflow(rp->rcb_socket);
 		error = ENOBUFS;
-		rp->rcb_socket->so_rcv.sb_overflowed++;
-	} else
+	} else {
+		sorwakeup(rp->rcb_socket);
 		error = 0;
-	sorwakeup(rp->rcb_socket);
+	}
 	return error;
 }
 
Index: sys/sys/event.h
===================================================================
RCS file: /cvsroot/src/sys/sys/event.h,v
retrieving revision 1.32
diff -u -p -r1.32 event.h
--- sys/sys/event.h	9 Jan 2018 03:31:13 -0000	1.32
+++ sys/sys/event.h	15 Mar 2018 16:34:13 -0000
@@ -99,7 +99,8 @@ _EV_SET(struct kevent *_kevp, uintptr_t 
 /*
  * data/hint flags for EVFILT_{READ|WRITE}, shared with userspace
  */
-#define	NOTE_LOWAT	0x0001U			/* low water mark */
+#define	NOTE_LOWAT	0x00000001U		/* low water mark */
+#define	NOTE_OVERFLOW	0x40000000U		/* buffer overflowed (>ELAST) */
 
 /*
  * data/hint flags for EVFILT_VNODE, shared with userspace
Index: sys/sys/socketvar.h
===================================================================
RCS file: /cvsroot/src/sys/sys/socketvar.h,v
retrieving revision 1.146
diff -u -p -r1.146 socketvar.h
--- sys/sys/socketvar.h	4 Jan 2018 01:42:25 -0000	1.146
+++ sys/sys/socketvar.h	15 Mar 2018 16:34:13 -0000
@@ -205,6 +205,7 @@ struct socket {
 					 */
 #define	SS_ISAPIPE 		0x1000	/* socket is implementing a pipe */
 #define	SS_NBIO			0x2000	/* socket is in non blocking I/O */
+#define	SS_RECVOVERFLOW		0x4000	/* socket receive buffer overflow */
 
 #ifdef _KERNEL
 
@@ -300,6 +301,7 @@ int	sofamily(const struct socket *);
 int	sobind(struct socket *, struct sockaddr *, struct lwp *);
 void	socantrcvmore(struct socket *);
 void	socantsendmore(struct socket *);
+void	soroverflow(struct socket *);
 int	soclose(struct socket *);
 int	soconnect(struct socket *, struct sockaddr *, struct lwp *);
 int	soconnect2(struct socket *, struct socket *);


Home | Main Index | Thread Index | Old Index