Date: Fri, 11 Jan 2008 18:32:57 -0500 From: John Baldwin <jhb@freebsd.org> To: Alfred Perlstein <alfred@freebsd.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org, Peter Wemm <peter@wemm.org> Subject: Re: cvs commit: src/sys/nfsclient nfs_socket.c Message-ID: <200801111832.58893.jhb@freebsd.org> In-Reply-To: <200801111154.45554.jhb@freebsd.org> References: <200801102336.m0ANa0mP035046@repoman.freebsd.org> <20080111023628.GB99258@elvis.mu.org> <200801111154.45554.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@wemm.org> [080110 17:39] wrote:
> > > On Jan 10, 2008 5:00 PM, Alfred Perlstein <alfred@freebsd.org> wrote:
> > > >
> > > > * John Baldwin <jhb@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200801111832.58893.jhb>