From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: sysctl_source(): move copyout(9) out of netlock
To: Alexander Bluhm <alexander.bluhm@gmx.net>
Cc: tech@openbsd.org
Date: Thu, 23 Jan 2025 23:10:37 +0300

On Thu, Jan 23, 2025 at 07:55:05PM +0100, Alexander Bluhm wrote:
> On Thu, Jan 23, 2025 at 08:15:25PM +0300, Vitaliy Makkoveev wrote:
> > Netlock required only to store data to local variable, the rest could be
> > done lockless. sysctl_source() is local to net/rtsock.c.
> 
> struct sockaddr sa is not enough to store all adreses families

Sorry, every time forget about it. This diff uses union of sockaddr_in
and sockaddr_in6 as temporary buffer. 

Index: sys/net/rtsock.c
===================================================================
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.377
diff -u -p -r1.377 rtsock.c
--- sys/net/rtsock.c	9 Jan 2025 18:20:29 -0000	1.377
+++ sys/net/rtsock.c	23 Jan 2025 20:05:14 -0000
@@ -2136,11 +2136,17 @@ sysctl_ifnames(struct walkarg *w)
 int
 sysctl_source(int af, u_int tableid, struct walkarg *w)
 {
+	union {
+		struct sockaddr_in in;
+#ifdef INET6
+		struct sockaddr_in6 in6;
+#endif
+	}		 buf;
 	struct sockaddr	*sa;
 	int		 size, error = 0;
 
-	sa = rtable_getsource(tableid, af);
-	if (sa) {
+	NET_LOCK_SHARED();
+	if ((sa = rtable_getsource(tableid, af)) != NULL) {
 		switch (sa->sa_family) {
 		case AF_INET:
 			size = sizeof(struct sockaddr_in);
@@ -2151,11 +2157,19 @@ sysctl_source(int af, u_int tableid, str
 			break;
 #endif
 		default:
-			return (0);
+			sa = NULL;
+			break;
 		}
+
+	}
+	if (sa != NULL)
+		memcpy(&buf, sa, size);
+	NET_UNLOCK_SHARED();
+
+	if (sa != NULL) {
 		w->w_needed += size;
 		if (w->w_where && w->w_needed <= w->w_given) {
-			if ((error = copyout(sa, w->w_where, size)))
+			if ((error = copyout(&buf, w->w_where, size)))
 				return (error);
 			w->w_where += size;
 		}
@@ -2236,7 +2250,6 @@ sysctl_rtable(int *name, u_int namelen, 
 		tableid = w.w_arg;
 		if (!rtable_exists(tableid))
 			return (ENOENT);
-		NET_LOCK_SHARED();
 		for (i = 1; i <= AF_MAX; i++) {
 			if (af != 0 && af != i)
 				continue;
@@ -2247,7 +2260,6 @@ sysctl_rtable(int *name, u_int namelen, 
 			if (error)
 				break;
 		}
-		NET_UNLOCK_SHARED();
 		break;
 	}
 	free(w.w_tmem, M_RTABLE, w.w_tmemsize);