On Mon, Apr 07, 2008 at 02:43:46PM -0400, J. Bruce Fields wrote:
> Thanks for the patch and the nice test case....
>
> On Fri, Apr 04, 2008 at 12:24:49PM +0200, Frank van Maarseveen wrote:
> > Occasionally we experience EACCES errors which are caused by the exportfs
> > reconnect_path() function called by the NFS server (NFSv3). The following
> > reproduces it reliably on the client.
>
> OK, so you're trying to use a directory that you previously descended
> into, but that you no longer have the right to look up?
Correct. Based on identity and permission bits the process should have
access but it no longer has.
>
> >
> > Compile cd-droppriv.c and make it setuid root:
> > --------
> > #include <stdio.h>
> > #include <stdarg.h>
> > #include <unistd.h>
> > #include <string.h>
> > #include <stdlib.h>
> > #include <errno.h>
> > #include <dirent.h>
> >
> > void die(const char *fmt, ...) __attribute__((format(printf, 1, 2), noreturn));
> > void die(const char *fmt, ...)
> > {
> > va_list ap;
> >
> > va_start(ap, fmt);
> > fprintf(stderr, "cd-droppriv: ");
> > vfprintf(stderr, fmt, ap);
> > va_end(ap);
> > exit(1);
> > }
> >
> > int main(int argc, char **argv)
> > {
> > if (chdir(argv[1]) == -1)
> > die("chdir: %s\n", strerror(errno));
> > if (setuid(getuid()) == -1)
> > die("setuid: %s\n", strerror(errno));
> > printf("Restart the NFS server then press <enter>.\n");
> > getchar();
> > if (opendir(".") == NULL)
> > die("opendir: %s\n", strerror(errno));
> > return 0;
> > }
> > --------
> >
> > As root, create a directory tree on the client. The export options on
> > the server for /mnt include no_root_squash and no_subtree_check:
> >
> > mkdir -p /mnt/a/b
> > chmod 700 /mnt/a
> > chmod 777 /mnt/a/b
> >
> > Run the program as non-root on the client:
> >
> > cd-droppriv /mnt/a/b
> >
> > and press <enter>. When the server is restarted before pressing <enter>
> > opendir() fails with EACCES:
> >
> > cd-droppriv: opendir: Permission denied
> >
> > This happens too when dentries are dropped on the server due to memory
> > pressure. The following seems to fix the problem (2.6.24.4):
> >
> > --- ./fs/exportfs/expfs.c.orig 2008-02-04 14:24:21.000000000 +0100
> > +++ ./fs/exportfs/expfs.c 2008-04-03 18:00:20.000000000 +0200
> > @@ -170,7 +170,7 @@ reconnect_path(struct vfsmount *mnt, str
> > }
> > dprintk("%s: found name: %s\n", __FUNCTION__, nbuf);
> > mutex_lock(&ppd->d_inode->i_mutex);
> > - npd = lookup_one_len(nbuf, ppd, strlen(nbuf));
> > + npd = lookup_one_noperm(nbuf, ppd);
>
> Anyone who depends on the "x" bit to control access to objects in an
> nfs-exported filesystem is already in trouble. We could do so for
> directories (at the expense of non-posix-like behavior such as what
> you've seen), but we probably can't for files. So I'm inclined to think
> this is the right thing to do.
Several file access issues have been fixed in the past. Redirecting
output of setuid-root programs (X server) when squash_root is in effect
and paging in execute-only files over NFS comes to my mind.
>
> The "DON'T USE THIS FUNCTION EVER, thanks." suggests we should at least
> consult the person who added that comment (cc'd) before adding a call to
> lookup_one_noperm(). (And if we decide to do this, we should make a
> note of this in that comment.)
yes.
>
> Also, I'm curious: could you explain how you hit this In Real Life,
> before you made this test case?
The NFS patch to deal with >16 groups on NFSv3 with AUTH_SYS I maintain
on www.frankvm.com/nfs-ngroups hit this problem first. Interactive shells
became unusable after a nightly run server mirroring script expelled
a lot of dentries from memory. The nfs-ngroups patch makes the client
picky about which groups to use in NFS requests in order to avoid the
16 groups limit.
>
> --b.
>
> > mutex_unlock(&ppd->d_inode->i_mutex);
> > if (IS_ERR(npd)) {
> > err = PTR_ERR(npd);
> > @@ -447,8 +447,7 @@ struct dentry *exportfs_decode_fh(struct
> > err = exportfs_get_name(mnt, target_dir, nbuf, result);
> > if (!err) {
> > mutex_lock(&target_dir->d_inode->i_mutex);
> > - nresult = lookup_one_len(nbuf, target_dir,
> > - strlen(nbuf));
> > + nresult = lookup_one_noperm(nbuf, target_dir);
> > mutex_unlock(&target_dir->d_inode->i_mutex);
> > if (!IS_ERR(nresult)) {
> > if (nresult->d_inode) {
> >
> > --
> > Frank
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Frank
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
