On Wed, 2017-01-25 at 08:52 -0600, Seth Forshee wrote: > On Wed, Jan 25, 2017 at 12:14:26AM +0000, Trond Myklebust wrote: > > Adding David Howells and Steve French as I believe both AFS and > > CIFS > > have the exact same requirements and NFS here. > > > > On Wed, 2017-01-25 at 12:56 +1300, Eric W. Biederman wrote: > > > Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> writes: > > > > > > > On Wed, 2017-01-25 at 12:28 +1300, Eric W. Biederman wrote: > > > > > With respect to nfs and automounts. > > > > > > > > > > Does NFS have different automount behavior based on the user > > > > > performing the automount? > > > > > > > > > > If NFS does not have different automount behavior depending > > > > > on > > > > > the > > > > > user > > > > > we just use the creds of the original mounter of NFS? > > > > > > > > > > If NFS does have different automount behavior depending on > > > > > the > > > > > user > > > > > (ouch!) we need to go through the call path and see where it > > > > > makes > > > > > sense to over ride things and where it does not. > > > > > > > > The reason why the NFS client creates a mountpoint is because > > > > on > > > > entering a directory, it detects that there is either a similar > > > > mountpoint on the server, or there is a referral (which acts > > > > sort > > > > of > > > > like a symlink, except it points to a path on one or more > > > > different > > > > NFS > > > > servers). > > > > Without that mountpoint, most things would work, but the user > > > > would > > > > end > > > > up seeing nasty non-posix features like duplicate inode > > > > numbers. > > > > > > > > We do not want to use any creds other than user creds here, > > > > because > > > > as > > > > far as the security model is concerned, the process is just > > > > crossing > > > > into an existing directory. > > > > > > But sget needs different creds. > > > > > > Because the user who authorizes the mounting of the filesystem is > > > different than the user who is crossing into the new filesystem. > > > > > > The local system now cares about that distinction even if the nfs > > > server > > > does not. > > > > Why? The filesystem is already mounted. We're creating a new > > mountpoint, but we could equally well just say 'sod that' and > > create an > > ordinary directory. The penalty would be aforementioned non-posix > > weirdness. > > > > > > > > > > Seth the fundamental problem with your patch was that you > > > > > were > > > > > patching > > > > > a location that is used for more just mounts. > > > > > > > > > > I am strongly wishing that we could just change > > > > > follow_automount > > > > > from: > > > > > > > > > > > > > > > old_cred = override_creds(&init_cred); > > > > > mnt = path->dentry->d_op->d_automount(path); > > > > > revert_creds(old_cred); > > > > > > > > > > to: > > > > > > > > > > old_cred = override_creds(path->mnt->mnt_sb->s_cred); > > > > > mnt = path->dentry->d_op->d_automount(path); > > > > > revert_creds(old_cred); > > > > > > > > > > And all will be well with nfs. That does remain possible. > > > > > > > > No. That would break permissions checking. See above. > > > > > > Then we need to look much harder at the permission checking > > > model of d_automount because we need to permission check against > > > both sets of creds. > > How about something like this? Essentially, stash the creds used at > mount time in the super block, then create a vfs_kern_automount() > function which overrides the currend creds then calls > vfs_kern_mount(). > > Only compile tested so far, and probably it should be split up into > several patches. > diff --git a/fs/namespace.c b/fs/namespace.c > index 487ba30bb5c6..da7e6dfa56cb 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -989,6 +989,21 @@ vfs_kern_mount(struct file_system_type *type, > int flags, const char *name, void > } > EXPORT_SYMBOL_GPL(vfs_kern_mount); > > +struct vfsmount * > +vfs_kern_automount(struct dentry *dentry, struct file_system_type > *type, > + int flags, const char *name, void *data) > +{ > + const struct cred *old_cred; > + struct vfsmount *mnt; > + > + old_cred = override_creds(dentry->d_sb->s_cred); > + mnt = vfs_kern_mount(type, flags, name, data); > + revert_creds(old_cred); > + > + return mnt; > +} > +EXPORT_SYMBOL_GPL(vfs_kern_automount); > + Nope. As I said, the call into the filesystem needs the _user_ creds. -- ��.n��������+%������w��{.n�����{��w���jg��������ݢj����G�������j:+v���w�m������w�������h�����٥