Download raw body.
unlock tcp drop sysctl
On Tue, Jan 07, 2025 at 12:47:30AM +0100, Alexander Bluhm wrote:
> Hi,
>
> Running TCP drop sysctl with shared net lock, allows to test socket
> locking directly from userland.
>
> mvs: I think you are right, moving sorele() from in_pcbsolock() to
> in_pcbsounlock() is a good idea.
>
> tcp_ident() uses shared net lock together with socket lock to call
> tcp_drop(). While there improve cosistency check of address family.
>
> ok?
>
> bluhm
mvs@ suggested a while ago to add _ref and _rele to the in_pcb..solock
functions. In addition to locking they also do refcounting.
As we protect tcp_drop() and tcp_close(), the inp->inp_socket in
in_pcbsounlock_rele() can be NULL now.
ok?
bluhm
Index: netinet/in_pcb.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
diff -u -p -r1.309 in_pcb.c
--- netinet/in_pcb.c 3 Jan 2025 12:56:15 -0000 1.309
+++ netinet/in_pcb.c 8 Jan 2025 16:22:37 -0000
@@ -623,7 +623,7 @@ in_pcbdetach(struct inpcb *inp)
}
struct socket *
-in_pcbsolock(struct inpcb *inp)
+in_pcbsolock_ref(struct inpcb *inp)
{
struct socket *so;
@@ -634,18 +634,18 @@ in_pcbsolock(struct inpcb *inp)
mtx_leave(&inp->inp_sofree_mtx);
if (so == NULL)
return NULL;
-
rw_enter_write(&so->so_lock);
- sorele(so);
-
return so;
}
void
-in_pcbsounlock(struct inpcb *inp, struct socket *so)
+in_pcbsounlock_rele(struct inpcb *inp, struct socket *so)
{
- KASSERT(inp->inp_socket == so);
+ if (so == NULL)
+ return;
+ KASSERT(inp->inp_socket == NULL || inp->inp_socket == so);
rw_exit_write(&so->so_lock);
+ sorele(so);
}
struct inpcb *
Index: netinet/in_pcb.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.h,v
diff -u -p -r1.163 in_pcb.h
--- netinet/in_pcb.h 5 Jan 2025 12:10:39 -0000 1.163
+++ netinet/in_pcb.h 8 Jan 2025 16:23:17 -0000
@@ -311,8 +311,8 @@ int in_pcbaddrisavail(const struct inpc
int in_pcbconnect(struct inpcb *, struct mbuf *);
void in_pcbdetach(struct inpcb *);
struct socket *
- in_pcbsolock(struct inpcb *);
-void in_pcbsounlock(struct inpcb *, struct socket *);
+ in_pcbsolock_ref(struct inpcb *);
+void in_pcbsounlock_rele(struct inpcb *, struct socket *);
struct inpcb *
in_pcbref(struct inpcb *);
void in_pcbunref(struct inpcb *);
Index: netinet/tcp_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
diff -u -p -r1.420 tcp_input.c
--- netinet/tcp_input.c 5 Jan 2025 12:18:48 -0000 1.420
+++ netinet/tcp_input.c 8 Jan 2025 16:24:01 -0000
@@ -3410,7 +3410,7 @@ syn_cache_timer(void *arg)
mtx_leave(&syn_cache_mtx);
NET_LOCK_SHARED();
- so = in_pcbsolock(inp);
+ so = in_pcbsolock_ref(inp);
if (so != NULL) {
now = tcp_now();
#ifdef TCP_ECN
@@ -3418,8 +3418,8 @@ syn_cache_timer(void *arg)
#endif
(void) syn_cache_respond(sc, NULL, now, do_ecn);
tcpstat_inc(tcps_sc_retransmitted);
- in_pcbsounlock(inp, so);
}
+ in_pcbsounlock_rele(inp, so);
NET_UNLOCK_SHARED();
in_pcbunref(inp);
Index: netinet/tcp_usrreq.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_usrreq.c,v
diff -u -p -r1.238 tcp_usrreq.c
--- netinet/tcp_usrreq.c 5 Jan 2025 12:23:38 -0000 1.238
+++ netinet/tcp_usrreq.c 8 Jan 2025 16:25:17 -0000
@@ -1122,15 +1122,13 @@ tcp_ident(void *oldp, size_t *oldlenp, v
int error = 0;
struct tcp_ident_mapping tir;
struct inpcb *inp;
- struct tcpcb *tp = NULL;
+ struct socket *so = NULL;
struct sockaddr_in *fin, *lin;
#ifdef INET6
struct sockaddr_in6 *fin6, *lin6;
struct in6_addr f6, l6;
#endif
- NET_ASSERT_LOCKED();
-
if (dodrop) {
if (oldp != NULL || *oldlenp != 0)
return (EINVAL);
@@ -1150,25 +1148,41 @@ tcp_ident(void *oldp, size_t *oldlenp, v
if ((error = copyin(oldp, &tir, sizeof (tir))) != 0 )
return (error);
}
+
+ NET_LOCK_SHARED();
+
switch (tir.faddr.ss_family) {
#ifdef INET6
case AF_INET6:
+ if (tir.laddr.ss_family != AF_INET6) {
+ NET_UNLOCK_SHARED();
+ return (EAFNOSUPPORT);
+ }
fin6 = (struct sockaddr_in6 *)&tir.faddr;
error = in6_embedscope(&f6, fin6, NULL, NULL);
- if (error)
+ if (error) {
+ NET_UNLOCK_SHARED();
return EINVAL; /*?*/
+ }
lin6 = (struct sockaddr_in6 *)&tir.laddr;
error = in6_embedscope(&l6, lin6, NULL, NULL);
- if (error)
+ if (error) {
+ NET_UNLOCK_SHARED();
return EINVAL; /*?*/
+ }
break;
#endif
case AF_INET:
+ if (tir.laddr.ss_family != AF_INET) {
+ NET_UNLOCK_SHARED();
+ return (EAFNOSUPPORT);
+ }
fin = (struct sockaddr_in *)&tir.faddr;
lin = (struct sockaddr_in *)&tir.laddr;
break;
default:
- return (EINVAL);
+ NET_UNLOCK_SHARED();
+ return (EAFNOSUPPORT);
}
switch (tir.faddr.ss_family) {
@@ -1187,11 +1201,20 @@ tcp_ident(void *oldp, size_t *oldlenp, v
}
if (dodrop) {
- if (inp && (tp = intotcpcb(inp)) &&
- ((inp->inp_socket->so_options & SO_ACCEPTCONN) == 0))
+ struct tcpcb *tp = NULL;
+
+ if (inp != NULL) {
+ so = in_pcbsolock_ref(inp);
+ if (so != NULL)
+ tp = intotcpcb(inp);
+ }
+ if (tp != NULL && !ISSET(so->so_options, SO_ACCEPTCONN))
tp = tcp_drop(tp, ECONNABORTED);
else
error = ESRCH;
+
+ in_pcbsounlock_rele(inp, so);
+ NET_UNLOCK_SHARED();
in_pcbunref(inp);
return (error);
}
@@ -1212,18 +1235,23 @@ tcp_ident(void *oldp, size_t *oldlenp, v
}
}
- if (inp != NULL && (inp->inp_socket->so_state & SS_CONNECTOUT)) {
- tir.ruid = inp->inp_socket->so_ruid;
- tir.euid = inp->inp_socket->so_euid;
+ if (inp != NULL)
+ so = in_pcbsolock_ref(inp);
+
+ if (so != NULL && ISSET(so->so_state, SS_CONNECTOUT)) {
+ tir.ruid = so->so_ruid;
+ tir.euid = so->so_euid;
} else {
tir.ruid = -1;
tir.euid = -1;
}
- *oldlenp = sizeof (tir);
- error = copyout((void *)&tir, oldp, sizeof (tir));
+ in_pcbsounlock_rele(inp, so);
+ NET_UNLOCK_SHARED();
in_pcbunref(inp);
- return (error);
+
+ *oldlenp = sizeof(tir);
+ return copyout(&tir, oldp, sizeof(tir));
}
int
@@ -1428,16 +1456,10 @@ tcp_sysctl(int *name, u_int namelen, voi
return (error);
case TCPCTL_IDENT:
- NET_LOCK();
- error = tcp_ident(oldp, oldlenp, newp, newlen, 0);
- NET_UNLOCK();
- return (error);
+ return tcp_ident(oldp, oldlenp, newp, newlen, 0);
case TCPCTL_DROP:
- NET_LOCK();
- error = tcp_ident(oldp, oldlenp, newp, newlen, 1);
- NET_UNLOCK();
- return (error);
+ return tcp_ident(oldp, oldlenp, newp, newlen, 1);
case TCPCTL_REASS_LIMIT:
NET_LOCK();
@@ -1506,9 +1528,8 @@ tcp_sysctl(int *name, u_int namelen, voi
return (error);
default:
- error = sysctl_bounded_arr(tcpctl_vars, nitems(tcpctl_vars),
+ return sysctl_bounded_arr(tcpctl_vars, nitems(tcpctl_vars),
name, namelen, oldp, oldlenp, newp, newlen);
- return (error);
}
/* NOTREACHED */
}
unlock tcp drop sysctl