Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 16 Jan 2024 18:31:35 GMT
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 289bee16be9c - main - sockets: remove dom_dispose and PR_RIGHTS
Message-ID:  <202401161831.40GIVZq6055963@gitrepo.freebsd.org>

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

URL: https://cgit.FreeBSD.org/src/commit/?id=289bee16be9c9e32451538fd5d31916b2a348947

commit 289bee16be9c9e32451538fd5d31916b2a348947
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2024-01-16 18:26:10 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2024-01-16 18:30:49 +0000

    sockets: remove dom_dispose and PR_RIGHTS
    
    Passing file descriptors (rights) via sockets is a feature specific to
    PF_UNIX only, so fully isolate the logic into uipc_usrreq.c.
    
    Reviewed by:            tuexen
    Differential Revision:  https://reviews.freebsd.org/D43414
---
 sys/kern/uipc_debug.c  |  5 -----
 sys/kern/uipc_socket.c | 15 ++-------------
 sys/kern/uipc_usrreq.c | 46 ++++++++++++++++++++++------------------------
 sys/sys/domain.h       |  2 --
 sys/sys/protosw.h      |  2 +-
 5 files changed, 25 insertions(+), 45 deletions(-)

diff --git a/sys/kern/uipc_debug.c b/sys/kern/uipc_debug.c
index 8ae523fa3403..1e699a10223e 100644
--- a/sys/kern/uipc_debug.c
+++ b/sys/kern/uipc_debug.c
@@ -242,7 +242,6 @@ db_print_domain(struct domain *d, const char *domain_name, int indent)
 
 	db_print_indent(indent);
 	db_printf("dom_externalize: %p   ", d->dom_externalize);
-	db_printf("dom_dispose: %p\n", d->dom_dispose);
 
 	db_print_indent(indent);
 	db_printf("dom_protosw: %p   ", d->dom_protosw);
@@ -278,10 +277,6 @@ db_print_prflags(short pr_flags)
 		db_printf("%sPR_WANTRCVD", comma ? ", " : "");
 		comma = 1;
 	}
-	if (pr_flags & PR_RIGHTS) {
-		db_printf("%sPR_RIGHTS", comma ? ", " : "");
-		comma = 1;
-	}
 	if (pr_flags & PR_IMPLOPCL) {
 		db_printf("%sPR_IMPLOPCL", comma ? ", " : "");
 		comma = 1;
diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index f61016d14e53..1626c6a7442d 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1201,10 +1201,6 @@ sofree(struct socket *so)
 		so->so_dtor(so);
 
 	VNET_SO_ASSERT(so);
-	if ((pr->pr_flags & PR_RIGHTS) && !SOLISTENING(so)) {
-		MPASS(pr->pr_domain->dom_dispose != NULL);
-		(*pr->pr_domain->dom_dispose)(so);
-	}
 	if (pr->pr_detach != NULL)
 		pr->pr_detach(so);
 
@@ -2981,7 +2977,6 @@ soshutdown(struct socket *so, enum shutdown_how how)
 void
 sorflush(struct socket *so)
 {
-	struct protosw *pr;
 	int error;
 
 	VNET_SO_ASSERT(so);
@@ -3001,14 +2996,8 @@ sorflush(struct socket *so)
 		return;
 	}
 
-	pr = so->so_proto;
-	if (pr->pr_flags & PR_RIGHTS) {
-		MPASS(pr->pr_domain->dom_dispose != NULL);
-		(*pr->pr_domain->dom_dispose)(so);
-	} else {
-		sbrelease(so, SO_RCV);
-		SOCK_IO_RECV_UNLOCK(so);
-	}
+	sbrelease(so, SO_RCV);
+	SOCK_IO_RECV_UNLOCK(so);
 
 }
 
diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
index 0460d2761e7c..554099586bbc 100644
--- a/sys/kern/uipc_usrreq.c
+++ b/sys/kern/uipc_usrreq.c
@@ -724,6 +724,9 @@ uipc_detach(struct socket *so)
 	vp = NULL;
 	vplock = NULL;
 
+	if (!SOLISTENING(so))
+		unp_dispose(so);
+
 	UNP_LINK_WLOCK();
 	LIST_REMOVE(unp, unp_link);
 	if (unp->unp_gcflag & UNPGC_DEAD)
@@ -1700,15 +1703,12 @@ uipc_shutdown(struct socket *so, enum shutdown_how how)
 
 	switch (how) {
 	case SHUT_RD:
-		/*
-		 * XXXGL: so far it is safe to call sorflush() on unix/dgram,
-		 * because PR_RIGHTS flag saves us from destructive sbrelease()
-		 * on our protocol specific buffers.
-		 */
-		sorflush(so);
+		socantrcvmore(so);
+		unp_dispose(so);
 		break;
 	case SHUT_RDWR:
-		sorflush(so);
+		socantrcvmore(so);
+		unp_dispose(so);
 		/* FALLTHROUGH */
 	case SHUT_WR:
 		UNP_PCB_LOCK(unp);
@@ -3193,7 +3193,8 @@ unp_gc(__unused void *arg, int pending)
 
 		so = unref[i]->f_data;
 		CURVNET_SET(so->so_vnet);
-		sorflush(so);
+		socantrcvmore(so);
+		unp_dispose(so);
 		CURVNET_RESTORE();
 	}
 
@@ -3250,15 +3251,15 @@ unp_dispose(struct socket *so)
 		 * XXXGL Mark sb with SBS_CANTRCVMORE.  This is needed to
 		 * prevent uipc_sosend_dgram() or unp_disconnect() adding more
 		 * data to the socket.
-		 * We are now in dom_dispose and it could be a call from
-		 * soshutdown() or from the final sofree().  The sofree() case
-		 * is simple as it guarantees that no more sends will happen,
-		 * however we can race with unp_disconnect() from our peer.
-		 * The shutdown(2) case is more exotic.  It would call into
-		 * dom_dispose() only if socket is SS_ISCONNECTED.  This is
-		 * possible if we did connect(2) on this socket and we also
-		 * had it bound with bind(2) and receive connections from other
-		 * sockets.  Because soshutdown() violates POSIX (see comment
+		 * We came here either through shutdown(2) or from the final
+		 * sofree().  The sofree() case is simple as it guarantees
+		 * that no more sends will happen, however we can race with
+		 * unp_disconnect() from our peer.  The shutdown(2) case is
+		 * more exotic.  It would call into unp_dispose() only if
+		 * socket is SS_ISCONNECTED.  This is possible if we did
+		 * connect(2) on this socket and we also had it bound with
+		 * bind(2) and receive connections from other sockets.
+		 * Because uipc_shutdown() violates POSIX (see comment
 		 * there) we will end up here shutting down our receive side.
 		 * Of course this will have affect not only on the peer we
 		 * connect(2)ed to, but also on all of the peers who had
@@ -3335,8 +3336,7 @@ unp_scan(struct mbuf *m0, void (*op)(struct filedescent **, int))
  */
 static struct protosw streamproto = {
 	.pr_type =		SOCK_STREAM,
-	.pr_flags =		PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS|
-				    PR_CAPATTACH,
+	.pr_flags =		PR_CONNREQUIRED | PR_WANTRCVD | PR_CAPATTACH,
 	.pr_ctloutput =		&uipc_ctloutput,
 	.pr_abort = 		uipc_abort,
 	.pr_accept =		uipc_peeraddr,
@@ -3362,8 +3362,7 @@ static struct protosw streamproto = {
 
 static struct protosw dgramproto = {
 	.pr_type =		SOCK_DGRAM,
-	.pr_flags =		PR_ATOMIC | PR_ADDR |PR_RIGHTS | PR_CAPATTACH |
-				    PR_SOCKBUF,
+	.pr_flags =		PR_ATOMIC | PR_ADDR | PR_CAPATTACH | PR_SOCKBUF,
 	.pr_ctloutput =		&uipc_ctloutput,
 	.pr_abort = 		uipc_abort,
 	.pr_accept =		uipc_peeraddr,
@@ -3391,8 +3390,8 @@ static struct protosw seqpacketproto = {
 	 * due to our use of sbappendaddr.  A new sbappend variants is needed
 	 * that supports both atomic record writes and control data.
 	 */
-	.pr_flags =		PR_ADDR|PR_ATOMIC|PR_CONNREQUIRED|
-				    PR_WANTRCVD|PR_RIGHTS|PR_CAPATTACH,
+	.pr_flags =		PR_ADDR | PR_ATOMIC | PR_CONNREQUIRED |
+				PR_WANTRCVD | PR_CAPATTACH,
 	.pr_ctloutput =		&uipc_ctloutput,
 	.pr_abort =		uipc_abort,
 	.pr_accept =		uipc_peeraddr,
@@ -3419,7 +3418,6 @@ static struct domain localdomain = {
 	.dom_family =		AF_LOCAL,
 	.dom_name =		"local",
 	.dom_externalize =	unp_externalize,
-	.dom_dispose =		unp_dispose,
 	.dom_nprotosw =		3,
 	.dom_protosw =		{
 		&streamproto,
diff --git a/sys/sys/domain.h b/sys/sys/domain.h
index 792376755750..45180aa06f80 100644
--- a/sys/sys/domain.h
+++ b/sys/sys/domain.h
@@ -54,8 +54,6 @@ struct domain {
 	int	(*dom_probe)(void);	/* check for support (optional) */
 	int	(*dom_externalize)	/* externalize access rights */
 		(struct mbuf *, struct mbuf **, int);
-	void	(*dom_dispose)		/* dispose of internalized rights */
-		(struct socket *);
 	struct rib_head *(*dom_rtattach)	/* initialize routing table */
 		(uint32_t);
 	void	(*dom_rtdetach)		/* clean up routing table */
diff --git a/sys/sys/protosw.h b/sys/sys/protosw.h
index 6fd21b947687..60d8ffc31362 100644
--- a/sys/sys/protosw.h
+++ b/sys/sys/protosw.h
@@ -157,7 +157,7 @@ struct protosw {
 #define	PR_ADDR		0x02		/* addresses given with messages */
 #define	PR_CONNREQUIRED	0x04		/* connection required by protocol */
 #define	PR_WANTRCVD	0x08		/* want PRU_RCVD calls */
-#define	PR_RIGHTS	0x10		/* passes capabilities */
+/* was	PR_RIGHTS	0x10		   passes capabilities */
 #define PR_IMPLOPCL	0x20		/* implied open/close */
 /* was	PR_LASTHDR	0x40		   enforce ipsec policy; last header */
 #define	PR_CAPATTACH	0x80		/* socket can attach in cap mode */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202401161831.40GIVZq6055963>