svn commit: r338192 - head/usr.sbin/nfsuserd

Rick Macklem rmacklem at FreeBSD.org
Wed Aug 22 12:20:11 UTC 2018


Author: rmacklem
Date: Wed Aug 22 12:20:10 2018
New Revision: 338192
URL: https://svnweb.freebsd.org/changeset/base/338192

Log:
  Revert r320757 since it can cause "excl->shared" panics.
  
  PR#230752 shows a panic where an nfsd thread tries to do soconnect() on
  the AF_LOCAL socket used by the nfsuserd while already holding an
  exclusive lock on it. I am not 100% sure how this happens, but since an
  AF_LOCAL socket is in the file system namespace it is conceivable that it
  could lock it and then attempt an upcall to the nfsuserd.
  However, reverting r320757 stops the nfsuserd from using an AF_LOCAL
  socket, so it should avoid any such panic().
  r320757 did fix a problem with running the nfsuserd when jails were
  enabled, but that can be dealt with less elegantly by allowing the
  use of an alternate address instead of 127.0.0.1.
  The gssd daemon also uses an AF_LOCAL socket, but it will do upcalls
  before the nfsd thread processes the RPC, so I think it should not
  be suseptible to this problem.
  
  PR:		230752

Modified:
  head/usr.sbin/nfsuserd/nfsuserd.c

Modified: head/usr.sbin/nfsuserd/nfsuserd.c
==============================================================================
--- head/usr.sbin/nfsuserd/nfsuserd.c	Wed Aug 22 11:56:51 2018	(r338191)
+++ head/usr.sbin/nfsuserd/nfsuserd.c	Wed Aug 22 12:20:10 2018	(r338192)
@@ -35,7 +35,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/mount.h>
 #include <sys/socket.h>
 #include <sys/socketvar.h>
-#include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/ucred.h>
 #include <sys/vnode.h>
@@ -44,7 +43,6 @@ __FBSDID("$FreeBSD$");
 #include <nfs/nfssvc.h>
 
 #include <rpc/rpc.h>
-#include <rpc/rpc_com.h>
 
 #include <fs/nfs/rpcv2.h>
 #include <fs/nfs/nfsproto.h>
@@ -75,9 +73,6 @@ static bool_t	xdr_getid(XDR *, caddr_t);
 static bool_t	xdr_getname(XDR *, caddr_t);
 static bool_t	xdr_retval(XDR *, caddr_t);
 
-#ifndef _PATH_NFSUSERDSOCK
-#define _PATH_NFSUSERDSOCK	"/var/run/nfsuserd.sock"
-#endif
 #define	MAXNAME		1024
 #define	MAXNFSUSERD	20
 #define	DEFNFSUSERD	4
@@ -97,7 +92,6 @@ uid_t defaultuid = 65534;
 u_char *defaultgroup = "nogroup";
 gid_t defaultgid = 65533;
 int verbose = 0, im_a_slave = 0, nfsuserdcnt = -1, forcestart = 0;
-int use_udpsock = 0;
 int defusertimeout = DEFUSERTIMEOUT, manage_gids = 0;
 pid_t slaves[MAXNFSUSERD];
 
@@ -109,17 +103,15 @@ main(int argc, char *argv[])
 	struct nfsd_idargs nid;
 	struct passwd *pwd;
 	struct group *grp;
-	int oldmask, one = 1, sock;
+	int sock, one = 1;
 	SVCXPRT *udptransp;
 	u_short portnum;
-	SVCXPRT *xprt;
 	sigset_t signew;
 	char hostname[MAXHOSTNAMELEN + 1], *cp;
 	struct addrinfo *aip, hints;
 	static uid_t check_dups[MAXUSERMAX];
 	gid_t grps[NGROUPS];
 	int ngroup;
-	struct sockaddr_un sun;
 
 	if (modfind("nfscommon") < 0) {
 		/* Not present in kernel, try loading it */
@@ -172,8 +164,6 @@ main(int argc, char *argv[])
 			forcestart = 1;
 		} else if (!strcmp(*argv, "-manage-gids")) {
 			manage_gids = 1;
-		} else if (!strcmp(*argv, "-use-udpsock")) {
-			use_udpsock = 1;
 		} else if (!strcmp(*argv, "-usermax")) {
 			if (argc == 1)
 				usage();
@@ -217,9 +207,6 @@ main(int argc, char *argv[])
 	}
 	if (nfsuserdcnt < 1)
 		nfsuserdcnt = DEFNFSUSERD;
-	if (use_udpsock == 0)
-		/* For AF_LOCAL socket, only allow one server daemon. */
-		nfsuserdcnt = 1;
 
 	/*
 	 * Strip off leading and trailing '.'s in domain name and map
@@ -258,93 +245,49 @@ main(int argc, char *argv[])
 	for (i = 0; i < nfsuserdcnt; i++)
 		slaves[i] = (pid_t)-1;
 
-	if (use_udpsock != 0) {
-		/*
-		 * Set up the service port to accept requests via UDP from
-		 * localhost (127.0.0.1).
-		 */
-		if ((sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP)) < 0)
-			err(1, "cannot create udp socket");
+	/*
+	 * Set up the service port to accept requests via UDP from
+	 * localhost (127.0.0.1).
+	 */
+	if ((sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP)) < 0)
+		err(1, "cannot create udp socket");
+
+	/*
+	 * Not sure what this does, so I'll leave it here for now.
+	 */
+	setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one));
 	
-		/*
-		 * Not sure what this does, so I'll leave it here for now.
-		 */
-		setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one));
-		
-		if ((udptransp = svcudp_create(sock)) == NULL)
-			err(1, "Can't set up socket");
-	
-		/*
-		 * By not specifying a protocol, it is linked into the
-		 * dispatch queue, but not registered with portmapper,
-		 * which is just what I want.
-		 */
-		if (!svc_register(udptransp, RPCPROG_NFSUSERD, RPCNFSUSERD_VERS,
-		    nfsuserdsrv, 0))
-			err(1, "Can't register nfsuserd");
-	
-		/*
-		 * Tell the kernel what my port# is.
-		 */
-		portnum = htons(udptransp->xp_port);
+	if ((udptransp = svcudp_create(sock)) == NULL)
+		err(1, "Can't set up socket");
+
+	/*
+	 * By not specifying a protocol, it is linked into the
+	 * dispatch queue, but not registered with portmapper,
+	 * which is just what I want.
+	 */
+	if (!svc_register(udptransp, RPCPROG_NFSUSERD, RPCNFSUSERD_VERS,
+	    nfsuserdsrv, 0))
+		err(1, "Can't register nfsuserd");
+
+	/*
+	 * Tell the kernel what my port# is.
+	 */
+	portnum = htons(udptransp->xp_port);
 #ifdef DEBUG
-		printf("portnum=0x%x\n", portnum);
+	printf("portnum=0x%x\n", portnum);
 #else
-		if (nfssvc(NFSSVC_NFSUSERDPORT, (caddr_t)&portnum) < 0) {
-			if (errno == EPERM)
-				fprintf(stderr, "Can't start nfsuserd when"
-				    " already running\nIf not running,"
-				    " use the -force option.\n");
-			else
-				fprintf(stderr,
-				    "Can't do nfssvc() to add socket\n");
-			exit(1);
+	if (nfssvc(NFSSVC_NFSUSERDPORT, (caddr_t)&portnum) < 0) {
+		if (errno == EPERM) {
+			fprintf(stderr,
+			    "Can't start nfsuserd when already running");
+			fprintf(stderr,
+			    " If not running, use the -force option.\n");
+		} else {
+			fprintf(stderr, "Can't do nfssvc() to add port\n");
 		}
-#endif
-	} else {
-		/* Use the AF_LOCAL socket. */
-		memset(&sun, 0, sizeof sun);
-		sun.sun_family = AF_LOCAL;
-		unlink(_PATH_NFSUSERDSOCK);
-		strcpy(sun.sun_path, _PATH_NFSUSERDSOCK);
-		sun.sun_len = SUN_LEN(&sun);
-		sock = socket(AF_LOCAL, SOCK_STREAM, 0);
-		if (sock < 0)
-			err(1, "Can't create local nfsuserd socket");
-		oldmask = umask(S_IXUSR | S_IRWXG | S_IRWXO);
-		if (bind(sock, (struct sockaddr *)&sun, sun.sun_len) < 0)
-			err(1, "Can't bind local nfsuserd socket");
-		umask(oldmask);
-		if (listen(sock, SOMAXCONN) < 0)
-			err(1, "Can't listen on local nfsuserd socket");
-		xprt = svc_vc_create(sock, RPC_MAXDATASIZE, RPC_MAXDATASIZE);
-		if (xprt == NULL)
-			err(1,
-			    "Can't create transport for local nfsuserd socket");
-		if (!svc_reg(xprt, RPCPROG_NFSUSERD, RPCNFSUSERD_VERS,
-		    nfsuserdsrv, NULL))
-			err(1,
-			    "Can't register service for local nfsuserd socket");
-	
-		/*
-		 * Tell the kernel what the socket's path is.
-		 */
-#ifdef DEBUG
-		printf("sockpath=%s\n", _PATH_NFSUSERDSOCK);
-#else
-		if (nfssvc(NFSSVC_NFSUSERDPORT | NFSSVC_NEWSTRUCT,
-		    _PATH_NFSUSERDSOCK) < 0) {
-			if (errno == EPERM)
-				fprintf(stderr, "Can't start nfsuserd when"
-				    " already running\nIf not running,"
-				    " use the -force option.\n");
-			else
-				fprintf(stderr,
-				    "Can't do nfssvc() to add socket\n");
-			exit(1);
-		}
-#endif
+		exit(1);
 	}
+#endif
 
 	pwd = getpwnam(defaultuser);
 	if (pwd)
@@ -519,25 +462,21 @@ nfsuserdsrv(struct svc_req *rqstp, SVCXPRT *transp)
 	gid_t grps[NGROUPS];
 	int ngroup;
 
-	if (use_udpsock != 0) {
-		/*
-		 * Only handle requests from 127.0.0.1 on a reserved port
-		 * number.  (Since a reserved port # at localhost implies a
-		 * client with local root, there won't be a security breach.
-		 * This is about the only case I can think of where a reserved
-		 * port # means something.)
-		 */
-		sport = ntohs(transp->xp_raddr.sin_port);
-		saddr = ntohl(transp->xp_raddr.sin_addr.s_addr);
-		if ((rqstp->rq_proc != NULLPROC && sport >= IPPORT_RESERVED) ||
-		    saddr != 0x7f000001) {
-			syslog(LOG_ERR, "req from ip=0x%x port=%d, consider"
-			    " using an AF_LOCAL socket\n", saddr, sport);
-			svcerr_weakauth(transp);
-			return;
-		}
+	/*
+	 * Only handle requests from 127.0.0.1 on a reserved port number.
+	 * (Since a reserved port # at localhost implies a client with
+	 *  local root, there won't be a security breach. This is about
+	 *  the only case I can think of where a reserved port # means
+	 *  something.)
+	 */
+	sport = ntohs(transp->xp_raddr.sin_port);
+	saddr = ntohl(transp->xp_raddr.sin_addr.s_addr);
+	if ((rqstp->rq_proc != NULLPROC && sport >= IPPORT_RESERVED) ||
+	    saddr != 0x7f000001) {
+		syslog(LOG_ERR, "req from ip=0x%x port=%d\n", saddr, sport);
+		svcerr_weakauth(transp);
+		return;
 	}
-
 	switch (rqstp->rq_proc) {
 	case NULLPROC:
 		if (!svc_sendreply(transp, (xdrproc_t)xdr_void, NULL))
@@ -781,7 +720,6 @@ static void
 usage(void)
 {
 
-	errx(1, "usage: nfsuserd [-usermax cache_size] [-usertimeout minutes]"
-	    " [-verbose] [-manage-gids] [-use-udpsock] [-domain domain_name]"
-	    " [n]");
+	errx(1,
+	    "usage: nfsuserd [-usermax cache_size] [-usertimeout minutes] [-verbose] [-manage-gids] [-domain domain_name] [n]");
 }


More information about the svn-src-head mailing list