On Thu, Jan 21, 2021 at 02:19:58PM +0100, Christian Brauner wrote:
> From: Christoph Hellwig <hch@xxxxxx>
>
> Enable idmapped mounts for xfs. This basically just means passing down
> the user_namespace argument from the VFS methods down to where it is
> passed to the relevant helpers.
>
> Note that full-filesystem bulkstat is not supported from inside idmapped
> mounts as it is an administrative operation that acts on the whole file
> system. The limitation is not applied to the bulkstat single operation
> that just operates on a single inode.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Christian Brauner <christian.brauner@xxxxxxxxxx>
> ---
> /* v2 */
>
> /* v3 */
>
> /* v4 */
>
> /* v5 */
> base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837
>
> /* v6 */
> unchanged
> base-commit: 19c329f6808995b142b3966301f217c831e7cf31
> ---
> fs/xfs/xfs_acl.c | 3 +--
> fs/xfs/xfs_file.c | 4 +++-
> fs/xfs/xfs_inode.c | 26 +++++++++++++++--------
> fs/xfs/xfs_inode.h | 16 +++++++++------
> fs/xfs/xfs_ioctl.c | 35 ++++++++++++++++++-------------
> fs/xfs/xfs_ioctl32.c | 6 ++++--
> fs/xfs/xfs_iops.c | 49 +++++++++++++++++++++++++-------------------
> fs/xfs/xfs_iops.h | 3 ++-
> fs/xfs/xfs_itable.c | 17 +++++++++++----
> fs/xfs/xfs_itable.h | 1 +
> fs/xfs/xfs_qm.c | 3 ++-
> fs/xfs/xfs_super.c | 2 +-
> fs/xfs/xfs_symlink.c | 5 +++--
> fs/xfs/xfs_symlink.h | 5 +++--
> 14 files changed, 110 insertions(+), 65 deletions(-)
<snip> Sorry for not noticing until after this went upstream, but...
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index 16ca97a7ff00..ca310a125d1e 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -54,10 +54,12 @@ struct xfs_bstat_chunk {
> STATIC int
> xfs_bulkstat_one_int(
> struct xfs_mount *mp,
> + struct user_namespace *mnt_userns,
> struct xfs_trans *tp,
> xfs_ino_t ino,
> struct xfs_bstat_chunk *bc)
> {
> + struct user_namespace *sb_userns = mp->m_super->s_user_ns;
> struct xfs_icdinode *dic; /* dinode core info pointer */
> struct xfs_inode *ip; /* incore inode pointer */
> struct inode *inode;
> @@ -86,8 +88,8 @@ xfs_bulkstat_one_int(
> */
> buf->bs_projectid = ip->i_d.di_projid;
> buf->bs_ino = ino;
> - buf->bs_uid = i_uid_read(inode);
> - buf->bs_gid = i_gid_read(inode);
> + buf->bs_uid = from_kuid(sb_userns, i_uid_into_mnt(mnt_userns, inode));
> + buf->bs_gid = from_kgid(sb_userns, i_gid_into_mnt(mnt_userns, inode));
> buf->bs_size = dic->di_size;
>
> buf->bs_nlink = inode->i_nlink;
> @@ -173,7 +175,8 @@ xfs_bulkstat_one(
> if (!bc.buf)
> return -ENOMEM;
>
> - error = xfs_bulkstat_one_int(breq->mp, NULL, breq->startino, &bc);
> + error = xfs_bulkstat_one_int(breq->mp, breq->mnt_userns, NULL,
> + breq->startino, &bc);
>
> kmem_free(bc.buf);
>
> @@ -194,9 +197,10 @@ xfs_bulkstat_iwalk(
> xfs_ino_t ino,
> void *data)
> {
> + struct xfs_bstat_chunk *bc = data;
> int error;
>
> - error = xfs_bulkstat_one_int(mp, tp, ino, data);
> + error = xfs_bulkstat_one_int(mp, bc->breq->mnt_userns, tp, ino, data);
> /* bulkstat just skips over missing inodes */
> if (error == -ENOENT || error == -EINVAL)
> return 0;
> @@ -239,6 +243,11 @@ xfs_bulkstat(
> };
> int error;
>
> + if (breq->mnt_userns != &init_user_ns) {
> + xfs_warn_ratelimited(breq->mp,
> + "bulkstat not supported inside of idmapped mounts.");
> + return -EINVAL;
Shouldn't this be -EPERM?
Or -EOPNOTSUPP?
Also, I'm not sure why bulkstat won't work in an idmapped mount but
bulkstat_single does? You can use the singleton version to stat inodes
that aren't inside the submount.
--D
> + }
> if (xfs_bulkstat_already_done(breq->mp, breq->startino))
> return 0;
>
> diff --git a/fs/xfs/xfs_itable.h b/fs/xfs/xfs_itable.h
> index 96a1e2a9be3f..7078d10c9b12 100644
> --- a/fs/xfs/xfs_itable.h
> +++ b/fs/xfs/xfs_itable.h
> @@ -8,6 +8,7 @@
> /* In-memory representation of a userspace request for batch inode data. */
> struct xfs_ibulk {
> struct xfs_mount *mp;
> + struct user_namespace *mnt_userns;
> void __user *ubuffer; /* user output buffer */
> xfs_ino_t startino; /* start with this inode */
> unsigned int icount; /* number of elements in ubuffer */
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index c134eb4aeaa8..1b7b1393cab2 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -787,7 +787,8 @@ xfs_qm_qino_alloc(
> return error;
>
> if (need_alloc) {
> - error = xfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0, 0, ipp);
> + error = xfs_dir_ialloc(&init_user_ns, &tp, NULL, S_IFREG, 1, 0,
> + 0, ipp);
> if (error) {
> xfs_trans_cancel(tp);
> return error;
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 813be879a5e5..e95c1eff95e0 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1912,7 +1912,7 @@ static struct file_system_type xfs_fs_type = {
> .init_fs_context = xfs_init_fs_context,
> .parameters = xfs_fs_parameters,
> .kill_sb = kill_block_super,
> - .fs_flags = FS_REQUIRES_DEV,
> + .fs_flags = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
> };
> MODULE_ALIAS_FS("xfs");
>
> diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
> index 1f43fd7f3209..77c8ea3229f1 100644
> --- a/fs/xfs/xfs_symlink.c
> +++ b/fs/xfs/xfs_symlink.c
> @@ -134,6 +134,7 @@ xfs_readlink(
>
> int
> xfs_symlink(
> + struct user_namespace *mnt_userns,
> struct xfs_inode *dp,
> struct xfs_name *link_name,
> const char *target_path,
> @@ -223,8 +224,8 @@ xfs_symlink(
> /*
> * Allocate an inode for the symlink.
> */
> - error = xfs_dir_ialloc(&tp, dp, S_IFLNK | (mode & ~S_IFMT), 1, 0,
> - prid, &ip);
> + error = xfs_dir_ialloc(mnt_userns, &tp, dp, S_IFLNK | (mode & ~S_IFMT),
> + 1, 0, prid, &ip);
> if (error)
> goto out_trans_cancel;
>
> diff --git a/fs/xfs/xfs_symlink.h b/fs/xfs/xfs_symlink.h
> index b1fa091427e6..2586b7e393f3 100644
> --- a/fs/xfs/xfs_symlink.h
> +++ b/fs/xfs/xfs_symlink.h
> @@ -7,8 +7,9 @@
>
> /* Kernel only symlink definitions */
>
> -int xfs_symlink(struct xfs_inode *dp, struct xfs_name *link_name,
> - const char *target_path, umode_t mode, struct xfs_inode **ipp);
> +int xfs_symlink(struct user_namespace *mnt_userns, struct xfs_inode *dp,
> + struct xfs_name *link_name, const char *target_path,
> + umode_t mode, struct xfs_inode **ipp);
> int xfs_readlink_bmap_ilocked(struct xfs_inode *ip, char *link);
> int xfs_readlink(struct xfs_inode *ip, char *link);
> int xfs_inactive_symlink(struct xfs_inode *ip);
> --
> 2.30.0
>
