Date: Wed, 22 Aug 2018 12:20:10 +0000 (UTC) From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r338192 - head/usr.sbin/nfsuserd Message-ID: <201808221220.w7MCKANm035378@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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]"); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201808221220.w7MCKANm035378>