Skip to content

Commit

Permalink
attr: handle idmapped mounts
Browse files Browse the repository at this point in the history
When file attributes are changed most filesystems rely on the
setattr_prepare(), setattr_copy(), and notify_change() helpers for
initialization and permission checking. Let them handle idmapped mounts.
If the inode is accessed through an idmapped mount map it into the
mount's user namespace. Afterwards the checks are identical to
non-idmapped mounts. If the initial user namespace is passed nothing
changes so non-idmapped mounts will see identical behavior as before.

Helpers that perform checks on the ia_uid and ia_gid fields in struct
iattr assume that ia_uid and ia_gid are intended values and have already
been mapped correctly at the userspace-kernelspace boundary as we
already do today. If the initial user namespace is passed nothing
changes so non-idmapped mounts will see identical behavior as before.

Link: https://lore.kernel.org/r/[email protected]
Cc: Christoph Hellwig <[email protected]>
Cc: David Howells <[email protected]>
Cc: Al Viro <[email protected]>
Cc: [email protected]
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Christian Brauner <[email protected]>
  • Loading branch information
Christian Brauner committed Jan 24, 2021
1 parent 21cb47b commit 2f221d6
Show file tree
Hide file tree
Showing 57 changed files with 206 additions and 137 deletions.
2 changes: 1 addition & 1 deletion arch/powerpc/platforms/cell/spufs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ spufs_setattr(struct dentry *dentry, struct iattr *attr)
if ((attr->ia_valid & ATTR_SIZE) &&
(attr->ia_size != inode->i_size))
return -EINVAL;
setattr_copy(inode, attr);
setattr_copy(&init_user_ns, inode, attr);
mark_inode_dirty(inode);
return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions drivers/base/devtmpfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ static int handle_create(const char *nodename, umode_t mode, kuid_t uid,
newattrs.ia_gid = gid;
newattrs.ia_valid = ATTR_MODE|ATTR_UID|ATTR_GID;
inode_lock(d_inode(dentry));
notify_change(dentry, &newattrs, NULL);
notify_change(&init_user_ns, dentry, &newattrs, NULL);
inode_unlock(d_inode(dentry));

/* mark as kernel-created inode */
Expand Down Expand Up @@ -328,7 +328,7 @@ static int handle_remove(const char *nodename, struct device *dev)
newattrs.ia_valid =
ATTR_UID|ATTR_GID|ATTR_MODE;
inode_lock(d_inode(dentry));
notify_change(dentry, &newattrs, NULL);
notify_change(&init_user_ns, dentry, &newattrs, NULL);
inode_unlock(d_inode(dentry));
err = vfs_unlink(d_inode(parent.dentry), dentry, NULL);
if (!err || err == -ENOENT)
Expand Down
4 changes: 2 additions & 2 deletions fs/9p/vfs_inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ static int v9fs_vfs_setattr(struct dentry *dentry, struct iattr *iattr)
struct p9_wstat wstat;

p9_debug(P9_DEBUG_VFS, "\n");
retval = setattr_prepare(dentry, iattr);
retval = setattr_prepare(&init_user_ns, dentry, iattr);
if (retval)
return retval;

Expand Down Expand Up @@ -1118,7 +1118,7 @@ static int v9fs_vfs_setattr(struct dentry *dentry, struct iattr *iattr)

v9fs_invalidate_inode_attr(d_inode(dentry));

setattr_copy(d_inode(dentry), iattr);
setattr_copy(&init_user_ns, d_inode(dentry), iattr);
mark_inode_dirty(d_inode(dentry));
return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions fs/9p/vfs_inode_dotl.c
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ int v9fs_vfs_setattr_dotl(struct dentry *dentry, struct iattr *iattr)

p9_debug(P9_DEBUG_VFS, "\n");

retval = setattr_prepare(dentry, iattr);
retval = setattr_prepare(&init_user_ns, dentry, iattr);
if (retval)
return retval;

Expand Down Expand Up @@ -590,7 +590,7 @@ int v9fs_vfs_setattr_dotl(struct dentry *dentry, struct iattr *iattr)
truncate_setsize(inode, iattr->ia_size);

v9fs_invalidate_inode_attr(inode);
setattr_copy(inode, iattr);
setattr_copy(&init_user_ns, inode, iattr);
mark_inode_dirty(inode);
if (iattr->ia_valid & ATTR_MODE) {
/* We also want to update ACL when we update mode bits */
Expand Down
2 changes: 1 addition & 1 deletion fs/adfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ adfs_notify_change(struct dentry *dentry, struct iattr *attr)
unsigned int ia_valid = attr->ia_valid;
int error;

error = setattr_prepare(dentry, attr);
error = setattr_prepare(&init_user_ns, dentry, attr);

/*
* we can't change the UID or GID of any file -
Expand Down
4 changes: 2 additions & 2 deletions fs/affs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ affs_notify_change(struct dentry *dentry, struct iattr *attr)

pr_debug("notify_change(%lu,0x%x)\n", inode->i_ino, attr->ia_valid);

error = setattr_prepare(dentry, attr);
error = setattr_prepare(&init_user_ns, dentry, attr);
if (error)
goto out;

Expand All @@ -249,7 +249,7 @@ affs_notify_change(struct dentry *dentry, struct iattr *attr)
affs_truncate(inode);
}

setattr_copy(inode, attr);
setattr_copy(&init_user_ns, inode, attr);
mark_inode_dirty(inode);

if (attr->ia_valid & ATTR_MODE)
Expand Down
119 changes: 90 additions & 29 deletions fs/attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,34 +18,63 @@
#include <linux/evm.h>
#include <linux/ima.h>

static bool chown_ok(const struct inode *inode, kuid_t uid)
/**
* chown_ok - verify permissions to chown inode
* @mnt_userns: user namespace of the mount @inode was found from
* @inode: inode to check permissions on
* @uid: uid to chown @inode to
*
* If the inode has been found through an idmapped mount the user namespace of
* the vfsmount must be passed through @mnt_userns. This function will then
* take care to map the inode according to @mnt_userns before checking
* permissions. On non-idmapped mounts or if permission checking is to be
* performed on the raw inode simply passs init_user_ns.
*/
static bool chown_ok(struct user_namespace *mnt_userns,
const struct inode *inode,
kuid_t uid)
{
if (uid_eq(current_fsuid(), inode->i_uid) &&
uid_eq(uid, inode->i_uid))
kuid_t kuid = i_uid_into_mnt(mnt_userns, inode);
if (uid_eq(current_fsuid(), kuid) && uid_eq(uid, kuid))
return true;
if (capable_wrt_inode_uidgid(&init_user_ns, inode, CAP_CHOWN))
if (capable_wrt_inode_uidgid(mnt_userns, inode, CAP_CHOWN))
return true;
if (uid_eq(inode->i_uid, INVALID_UID) &&
if (uid_eq(kuid, INVALID_UID) &&
ns_capable(inode->i_sb->s_user_ns, CAP_CHOWN))
return true;
return false;
}

static bool chgrp_ok(const struct inode *inode, kgid_t gid)
/**
* chgrp_ok - verify permissions to chgrp inode
* @mnt_userns: user namespace of the mount @inode was found from
* @inode: inode to check permissions on
* @gid: gid to chown @inode to
*
* If the inode has been found through an idmapped mount the user namespace of
* the vfsmount must be passed through @mnt_userns. This function will then
* take care to map the inode according to @mnt_userns before checking
* permissions. On non-idmapped mounts or if permission checking is to be
* performed on the raw inode simply passs init_user_ns.
*/
static bool chgrp_ok(struct user_namespace *mnt_userns,
const struct inode *inode, kgid_t gid)
{
if (uid_eq(current_fsuid(), inode->i_uid) &&
(in_group_p(gid) || gid_eq(gid, inode->i_gid)))
kgid_t kgid = i_gid_into_mnt(mnt_userns, inode);
if (uid_eq(current_fsuid(), i_uid_into_mnt(mnt_userns, inode)) &&
(in_group_p(gid) || gid_eq(gid, kgid)))
return true;
if (capable_wrt_inode_uidgid(&init_user_ns, inode, CAP_CHOWN))
if (capable_wrt_inode_uidgid(mnt_userns, inode, CAP_CHOWN))
return true;
if (gid_eq(inode->i_gid, INVALID_GID) &&
if (gid_eq(kgid, INVALID_GID) &&
ns_capable(inode->i_sb->s_user_ns, CAP_CHOWN))
return true;
return false;
}

/**
* setattr_prepare - check if attribute changes to a dentry are allowed
* @mnt_userns: user namespace of the mount the inode was found from
* @dentry: dentry to check
* @attr: attributes to change
*
Expand All @@ -55,10 +84,17 @@ static bool chgrp_ok(const struct inode *inode, kgid_t gid)
* SGID bit from mode if user is not allowed to set it. Also file capabilities
* and IMA extended attributes are cleared if ATTR_KILL_PRIV is set.
*
* If the inode has been found through an idmapped mount the user namespace of
* the vfsmount must be passed through @mnt_userns. This function will then
* take care to map the inode according to @mnt_userns before checking
* permissions. On non-idmapped mounts or if permission checking is to be
* performed on the raw inode simply passs init_user_ns.
*
* Should be called as the first thing in ->setattr implementations,
* possibly after taking additional locks.
*/
int setattr_prepare(struct dentry *dentry, struct iattr *attr)
int setattr_prepare(struct user_namespace *mnt_userns, struct dentry *dentry,
struct iattr *attr)
{
struct inode *inode = d_inode(dentry);
unsigned int ia_valid = attr->ia_valid;
Expand All @@ -78,27 +114,27 @@ int setattr_prepare(struct dentry *dentry, struct iattr *attr)
goto kill_priv;

/* Make sure a caller can chown. */
if ((ia_valid & ATTR_UID) && !chown_ok(inode, attr->ia_uid))
if ((ia_valid & ATTR_UID) && !chown_ok(mnt_userns, inode, attr->ia_uid))
return -EPERM;

/* Make sure caller can chgrp. */
if ((ia_valid & ATTR_GID) && !chgrp_ok(inode, attr->ia_gid))
if ((ia_valid & ATTR_GID) && !chgrp_ok(mnt_userns, inode, attr->ia_gid))
return -EPERM;

/* Make sure a caller can chmod. */
if (ia_valid & ATTR_MODE) {
if (!inode_owner_or_capable(&init_user_ns, inode))
if (!inode_owner_or_capable(mnt_userns, inode))
return -EPERM;
/* Also check the setgid bit! */
if (!in_group_p((ia_valid & ATTR_GID) ? attr->ia_gid :
inode->i_gid) &&
!capable_wrt_inode_uidgid(&init_user_ns, inode, CAP_FSETID))
if (!in_group_p((ia_valid & ATTR_GID) ? attr->ia_gid :
i_gid_into_mnt(mnt_userns, inode)) &&
!capable_wrt_inode_uidgid(mnt_userns, inode, CAP_FSETID))
attr->ia_mode &= ~S_ISGID;
}

/* Check for setting the inode time. */
if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET | ATTR_TIMES_SET)) {
if (!inode_owner_or_capable(&init_user_ns, inode))
if (!inode_owner_or_capable(mnt_userns, inode))
return -EPERM;
}

Expand Down Expand Up @@ -162,20 +198,33 @@ EXPORT_SYMBOL(inode_newsize_ok);

/**
* setattr_copy - copy simple metadata updates into the generic inode
* @mnt_userns: user namespace of the mount the inode was found from
* @inode: the inode to be updated
* @attr: the new attributes
*
* setattr_copy must be called with i_mutex held.
*
* setattr_copy updates the inode's metadata with that specified
* in attr. Noticeably missing is inode size update, which is more complex
* in attr on idmapped mounts. If file ownership is changed setattr_copy
* doesn't map ia_uid and ia_gid. It will asssume the caller has already
* provided the intended values. Necessary permission checks to determine
* whether or not the S_ISGID property needs to be removed are performed with
* the correct idmapped mount permission helpers.
* Noticeably missing is inode size update, which is more complex
* as it requires pagecache updates.
*
* If the inode has been found through an idmapped mount the user namespace of
* the vfsmount must be passed through @mnt_userns. This function will then
* take care to map the inode according to @mnt_userns before checking
* permissions. On non-idmapped mounts or if permission checking is to be
* performed on the raw inode simply passs init_user_ns.
*
* The inode is not marked as dirty after this operation. The rationale is
* that for "simple" filesystems, the struct inode is the inode storage.
* The caller is free to mark the inode dirty afterwards if needed.
*/
void setattr_copy(struct inode *inode, const struct iattr *attr)
void setattr_copy(struct user_namespace *mnt_userns, struct inode *inode,
const struct iattr *attr)
{
unsigned int ia_valid = attr->ia_valid;

Expand All @@ -191,9 +240,9 @@ void setattr_copy(struct inode *inode, const struct iattr *attr)
inode->i_ctime = attr->ia_ctime;
if (ia_valid & ATTR_MODE) {
umode_t mode = attr->ia_mode;

if (!in_group_p(inode->i_gid) &&
!capable_wrt_inode_uidgid(&init_user_ns, inode, CAP_FSETID))
kgid_t kgid = i_gid_into_mnt(mnt_userns, inode);
if (!in_group_p(kgid) &&
!capable_wrt_inode_uidgid(mnt_userns, inode, CAP_FSETID))
mode &= ~S_ISGID;
inode->i_mode = mode;
}
Expand All @@ -202,6 +251,7 @@ EXPORT_SYMBOL(setattr_copy);

/**
* notify_change - modify attributes of a filesytem object
* @mnt_userns: user namespace of the mount the inode was found from
* @dentry: object affected
* @attr: new attributes
* @delegated_inode: returns inode, if the inode is delegated
Expand All @@ -214,13 +264,23 @@ EXPORT_SYMBOL(setattr_copy);
* retry. Because breaking a delegation may take a long time, the
* caller should drop the i_mutex before doing so.
*
* If file ownership is changed notify_change() doesn't map ia_uid and
* ia_gid. It will asssume the caller has already provided the intended values.
*
* Alternatively, a caller may pass NULL for delegated_inode. This may
* be appropriate for callers that expect the underlying filesystem not
* to be NFS exported. Also, passing NULL is fine for callers holding
* the file open for write, as there can be no conflicting delegation in
* that case.
*
* If the inode has been found through an idmapped mount the user namespace of
* the vfsmount must be passed through @mnt_userns. This function will then
* take care to map the inode according to @mnt_userns before checking
* permissions. On non-idmapped mounts or if permission checking is to be
* performed on the raw inode simply passs init_user_ns.
*/
int notify_change(struct dentry * dentry, struct iattr * attr, struct inode **delegated_inode)
int notify_change(struct user_namespace *mnt_userns, struct dentry *dentry,
struct iattr *attr, struct inode **delegated_inode)
{
struct inode *inode = dentry->d_inode;
umode_t mode = inode->i_mode;
Expand All @@ -243,9 +303,8 @@ int notify_change(struct dentry * dentry, struct iattr * attr, struct inode **de
if (IS_IMMUTABLE(inode))
return -EPERM;

if (!inode_owner_or_capable(&init_user_ns, inode)) {
error = inode_permission(&init_user_ns, inode,
MAY_WRITE);
if (!inode_owner_or_capable(mnt_userns, inode)) {
error = inode_permission(mnt_userns, inode, MAY_WRITE);
if (error)
return error;
}
Expand Down Expand Up @@ -321,9 +380,11 @@ int notify_change(struct dentry * dentry, struct iattr * attr, struct inode **de
/* Don't allow modifications of files with invalid uids or
* gids unless those uids & gids are being made valid.
*/
if (!(ia_valid & ATTR_UID) && !uid_valid(inode->i_uid))
if (!(ia_valid & ATTR_UID) &&
!uid_valid(i_uid_into_mnt(mnt_userns, inode)))
return -EOVERFLOW;
if (!(ia_valid & ATTR_GID) && !gid_valid(inode->i_gid))
if (!(ia_valid & ATTR_GID) &&
!gid_valid(i_gid_into_mnt(mnt_userns, inode)))
return -EOVERFLOW;

error = security_inode_setattr(dentry, attr);
Expand Down
4 changes: 2 additions & 2 deletions fs/btrfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -5054,7 +5054,7 @@ static int btrfs_setattr(struct dentry *dentry, struct iattr *attr)
if (btrfs_root_readonly(root))
return -EROFS;

err = setattr_prepare(dentry, attr);
err = setattr_prepare(&init_user_ns, dentry, attr);
if (err)
return err;

Expand All @@ -5065,7 +5065,7 @@ static int btrfs_setattr(struct dentry *dentry, struct iattr *attr)
}

if (attr->ia_valid) {
setattr_copy(inode, attr);
setattr_copy(&init_user_ns, inode, attr);
inode_inc_iversion(inode);
err = btrfs_dirty_inode(inode);

Expand Down
4 changes: 2 additions & 2 deletions fs/cachefiles/interface.c
Original file line number Diff line number Diff line change
Expand Up @@ -470,14 +470,14 @@ static int cachefiles_attr_changed(struct fscache_object *_object)
_debug("discard tail %llx", oi_size);
newattrs.ia_valid = ATTR_SIZE;
newattrs.ia_size = oi_size & PAGE_MASK;
ret = notify_change(object->backer, &newattrs, NULL);
ret = notify_change(&init_user_ns, object->backer, &newattrs, NULL);
if (ret < 0)
goto truncate_failed;
}

newattrs.ia_valid = ATTR_SIZE;
newattrs.ia_size = ni_size;
ret = notify_change(object->backer, &newattrs, NULL);
ret = notify_change(&init_user_ns, object->backer, &newattrs, NULL);

truncate_failed:
inode_unlock(d_inode(object->backer));
Expand Down
2 changes: 1 addition & 1 deletion fs/ceph/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -2247,7 +2247,7 @@ int ceph_setattr(struct dentry *dentry, struct iattr *attr)
if (ceph_snap(inode) != CEPH_NOSNAP)
return -EROFS;

err = setattr_prepare(dentry, attr);
err = setattr_prepare(&init_user_ns, dentry, attr);
if (err != 0)
return err;

Expand Down
Loading

0 comments on commit 2f221d6

Please sign in to comment.