Re: [REVIEW][PATCH] fs: Better permission checking for submounts

[Seth Forshee <seth.forshee@canonical.com>]

On Wed, Feb 01, 2017 at 07:38:17PM +1300, Eric W. Biederman wrote:
> 
> To support unprivileged users mounting filesystems two permission
> checks have to be performed: a test to see if the user allowed to
> create a mount in the mount namespace, and a test to see if
> the user is allowed to access the specified filesystem.
> 
> The automount case is special in that mounting the original filesystem
> grants permission to mount the sub-filesystems, to any user who
> happens to stumble across the their mountpoint and satisfies the
> ordinary filesystem permission checks.
> 
> Attempting to handle the automount case by using override_creds
> almost works.  It preserves the idea that permission to mount
> the original filesystem is permission to mount the sub-filesystem.
> Unfortunately using override_creds messes up the filesystems
> ordinary permission checks.
> 
> Solve this by being explicit that a mount is a submount by introducing
> vfs_submount, and using it where appropriate.
> 
> vfs_submount uses a new mount internal mount flags MS_SUBMOUNT, to let
> sget and friends know that a mount is a submount so they can take appropriate
> action.
> 
> sget and sget_userns are modified to not perform any permission checks
> on submounts.
> 
> follow_automount is modified to stop using override_creds as that
> has proven problemantic.
> 
> do_mount is modified to always remove the new MS_SUBMOUNT flag so
> that we know userspace will never by able to specify it.
> 
> autofs4 is modified to stop using current_real_cred that was put in
> there to handle the previous version of submount permission checking.
> 
> cifs is modified to pass the mountpoint all of the way down to vfs_submount.
> 
> debugfs is modified to pass the mountpoint all of the way down to
> trace_automount by adding a new parameter.  To make this change easier
> a new typedef debugfs_automount_t is introduced to capture the type of
> the debugfs automount function.
> 
> Cc: stable@vger.kernel.org
> Fixes: 069d5ac9ae0d ("autofs:  Fix automounts by using current_real_cred()->uid")
> Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems creds")
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

Looks good to me. I also got testing from the user who reported the bug
to us, and it does fix his nfs submount problem.

Reviewed-by: Seth Forshee <seth.forshee@canonical.com>