From: Christoph Hellwig <hch@infradead.org> Date: 2021-01-19 22:56:09
On Tue, Jan 12, 2021 at 11:00:53PM +0100, Christian Brauner wrote:
The two helpers inode_permission() and generic_permission() are used by
the vfs to perform basic permission checking by verifying that the
caller is privileged over an inode. In order to handle idmapped mounts
we extend the two helpers with an additional user namespace argument.
On idmapped mounts the two helpers will make sure to map the inode
according to the mount's user namespace and then peform identical
permission checks to inode_permission() and generic_permission(). If the
initial user namespace is passed nothing changes so non-idmapped mounts
will see identical behavior as before.
More long lines in here.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
From: Christoph Hellwig <hch@infradead.org> Date: 2021-01-19 22:56:10
On Tue, Jan 12, 2021 at 11:00:52PM +0100, Christian Brauner wrote:
In order to determine whether a caller holds privilege over a given
inode the capability framework exposes the two helpers
privileged_wrt_inode_uidgid() and capable_wrt_inode_uidgid(). The former
verifies that the inode has a mapping in the caller's user namespace and
the latter additionally verifies that the caller has the requested
capability in their current user namespace.
If the inode is accessed through an idmapped mount we simply need to map
it according to the mount's user namespace. Afterwards the checks are
identical to non-idmapped inodes. If the initial user namespace is
passed all operations are a nop so non-idmapped mounts will not see a
change in behavior and will also not see any performance impact.
This adds a bunch of pointless > 80 char lines, that would be nice to
fix up.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
From: Christian Brauner <hidden> Date: 2021-01-20 12:39:44
On Tue, Jan 19, 2021 at 04:05:00PM +0100, Jann Horn wrote:
On Wed, Jan 13, 2021 at 1:52 AM Christian Brauner
[off-list ref] wrote:
quoted
Add a simple helper to retrieve the user namespace associated with the
vfsmount of a file. Christoph correctly points out that this makes
codepaths (e.g. ioctls) way easier to follow that would otherwise
dereference via mnt_user_ns(file->f_path.mnt).
In order to make file_user_ns() static inline we'd need to include
mount.h in either file.h or fs.h which seems undesirable so let's simply
not force file_user_ns() to be inline.
That name is confusing to me, because when I think of "the userns of a
file", it's file->f_cred->user_ns. There are a bunch of places that
look at that, as you can see from grepping for "f_cred->user_ns".
If you really want this to be a separate helper, can you maybe give it
a clearer name? file_mnt_user_ns(), or something like that, idk.