cvs commit: src/sys/nfsclient nfs_socket.c

[John Baldwin]

On Friday 11 January 2008 11:54:44 am John Baldwin wrote:
> On Thursday 10 January 2008 09:36:28 pm Alfred Perlstein wrote:
> > * Peter Wemm <peter at wemm.org> [080110 17:39] wrote:
> > > On Jan 10, 2008 5:00 PM, Alfred Perlstein <alfred at freebsd.org> wrote:
> > > >
> > > > * John Baldwin <jhb at FreeBSD.org> [080110 15:33] wrote:
> > > > > jhb         2008-01-10 23:36:00 UTC
> > > > >
> > > > >   FreeBSD src repository
> > > > >
> > > > >   Modified files:
> > > > >     sys/nfsclient        nfs_socket.c
> > > > >   Log:
> > > > >   Pass curthread to various socket routines (socreate(), sobind(), and
> > > > >   soconnect()) instead of &thread0 when establishing a connection to the NFS
> > > > >   server.  Otherwise inconsistent credentials may be used when setting up
> > > > >   the NFS socket.
> > > >
> > > > I'm not sure, but I think this may be a regression, I seem to recall
> > > > that a long time ago it was switched to &thread0 because otherwise
> > > > certain operations can fail due to curthread not running as root.
> > > 
> > > That's my recollection too.  For example, when nfs is configured to
> > > bind to a priviliged local port for making queries or connections, it
> > > had to be done as root.  With tcp mounts, the connection can be
> > > dropped and a reconnect required at any time.
> > 
> > This could be implemented by a handoff to a thread that does the
> > appropriate setuid call beforehand, or perhaps the credential
> > inconsistencies can be further expained or fixed.
> 
> The problem case I have is doing a mount inside of a jail.  The socket's
> credential is jailed but thread0's credential is not, and you end up with
> odd behavior where sobind() treats the socket as non-jailed (and thus only
> binds the local port and not the local IP address) but soconnect() treats
> the socket as jailed and fails with EINVAL when it sees a partially bound
> socket.  (sobind() of a jailed socket sets both the port and IP.)  What I
> can do as a workaround is to change curthread's ucred to be the NFS mount's
> credential during nfs_connect() by fiddling with td_ucred.  It would be
> safe as it wouldn't affect other threads even in the same process and the
> current thread isn't going to be doing anything else until the function
> returns with the restored credentials, just hackish.

I have a patch to do this and it works both inside and out of jails for both
UDP and TCP mounts.  I used tcpdrop on the server in each case to force the
client to reconnect on a different socket and that worked ok while doing
a ls -l as a mere mortal.  I also verified that this test case (dropping the
existing TCP connection) caused the ls process to hang in "nfscon" with the
change I committed yesterday.  This is relative to what is in HEAD now:

Index: nfs_socket.c
===================================================================
RCS file: /host/cvs/usr/cvs/src/sys/nfsclient/nfs_socket.c,v
retrieving revision 1.156
diff -u -r1.156 nfs_socket.c
--- nfs_socket.c	10 Jan 2008 23:36:00 -0000	1.156
+++ nfs_socket.c	11 Jan 2008 17:20:30 -0000
@@ -264,7 +264,19 @@
 	int error, rcvreserve, sndreserve;
 	int pktscale;
 	struct sockaddr *saddr;
-	struct thread *td = curthread; /* only used for socreate and sobind */
+	struct ucred *origcred;
+	struct thread *td = curthread;
+
+	/*
+	 * We need to establish the socket using the credentials of
+	 * the mountpoint.  Some parts of this process (such as
+	 * sobind() and soconnect()) will use the curent thread's
+	 * credential instead of the socket credential.  To work
+	 * around this, temporarily change the current thread's
+	 * credential to that of the mountpoint.
+	 */
+	origcred = td->td_ucred;
+	td->td_ucred = nmp->nm_mountp->mnt_cred;
 
 	if (nmp->nm_sotype == SOCK_STREAM) {
 		mtx_lock(&nmp->nm_mtx);
@@ -453,6 +465,9 @@
 	so->so_snd.sb_flags |= SB_NOINTR;
 	SOCKBUF_UNLOCK(&so->so_snd);
 
+	/* Restore current thread's credentials. */
+	td->td_ucred = origcred;
+
 	mtx_lock(&nmp->nm_mtx);
 	/* Initialize other non-zero congestion variables */
 	nfs_init_rtt(nmp);
@@ -463,6 +478,9 @@
 	return (0);
 
 bad:
+	/* Restore current thread's credentials. */
+	td->td_ucred = origcred;
+
 	nfs_disconnect(nmp);
 	return (error);
 }


-- 
John Baldwin