Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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�����٥




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux