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 <[EMAIL PROTECTED]> [080110 17:39] wrote:
> > > On Jan 10, 2008 5:00 PM, Alfred Perlstein <[EMAIL PROTECTED]> wrote:
> > > >
> > > > * John Baldwin <[EMAIL PROTECTED]> [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
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to