[bpf-next,v2] bpf: Support uid and gid when mounting bpffs

Commit Message

Jie Jiang Dec. 6, 2023, 7:36 a.m. UTC

Parse uid and gid in bpf_parse_param() so that they can be passed in as
the `data` parameter when mount() bpffs. This will be useful when we
want to control which user/group has the control to the mounted bpffs,
otherwise a separate chown() call will be needed.

Signed-off-by: Jie Jiang <[e-mail]>
---
v1 -> v2: Add additional validation in bpf_parse_param() for if the
  requested uid/gid is representable in the fs's idmapping.

 kernel/bpf/inode.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

Comments

Christian Brauner Dec. 6, 2023, 10:55 a.m. UTC | #1

On Wed, Dec 06, 2023 at 07:36:24AM +0000, Jie Jiang wrote:
> Parse uid and gid in bpf_parse_param() so that they can be passed in as
> the `data` parameter when mount() bpffs. This will be useful when we
> want to control which user/group has the control to the mounted bpffs,
> otherwise a separate chown() call will be needed.
> 
> Signed-off-by: Jie Jiang <[e-mail]>
> ---

Yes, looks good now,
Acked-by: Christian Brauner <[e-mail]>

Andrii Nakryiko Dec. 6, 2023, 5:21 p.m. UTC | #2

On Tue, Dec 5, 2023 at 11:36 PM Jie Jiang <[e-mail]> wrote:
>
> Parse uid and gid in bpf_parse_param() so that they can be passed in as
> the `data` parameter when mount() bpffs. This will be useful when we
> want to control which user/group has the control to the mounted bpffs,
> otherwise a separate chown() call will be needed.
>
> Signed-off-by: Jie Jiang <[e-mail]>
> ---
> v1 -> v2: Add additional validation in bpf_parse_param() for if the
>   requested uid/gid is representable in the fs's idmapping.
>
>  kernel/bpf/inode.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 50 insertions(+), 2 deletions(-)
>

LGTM, but I want to point out that this will conflict with the BPF
token series ([0]), so depending which one goes in first, the other
will have to be rebased.

Acked-by: Andrii Nakryiko <[e-mail]>


  [0] https://patchwork.kernel.org/project/netdevbpf/list/?series=805707&state=*

>
> diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
> index 1aafb2ff2e953..5bc79535d3357 100644
> --- a/kernel/bpf/inode.c
> +++ b/kernel/bpf/inode.c
> @@ -599,8 +599,15 @@ EXPORT_SYMBOL(bpf_prog_get_type_path);
>   */
>  static int bpf_show_options(struct seq_file *m, struct dentry *root)
>  {
> -       umode_t mode = d_inode(root)->i_mode & S_IALLUGO & ~S_ISVTX;
> -
> +       struct inode *inode = d_inode(root);
> +       umode_t mode = inode->i_mode & S_IALLUGO & ~S_ISVTX;
> +
> +       if (!uid_eq(inode->i_uid, GLOBAL_ROOT_UID))
> +               seq_printf(m, ",uid=%u",
> +                          from_kuid_munged(&init_user_ns, inode->i_uid));
> +       if (!gid_eq(inode->i_gid, GLOBAL_ROOT_GID))
> +               seq_printf(m, ",gid=%u",
> +                          from_kgid_munged(&init_user_ns, inode->i_gid));
>         if (mode != S_IRWXUGO)
>                 seq_printf(m, ",mode=%o", mode);
>         return 0;
> @@ -625,15 +632,21 @@ static const struct super_operations bpf_super_ops = {
>  };
>
>  enum {
> +       OPT_UID,
> +       OPT_GID,
>         OPT_MODE,
>  };
>
>  static const struct fs_parameter_spec bpf_fs_parameters[] = {
> +       fsparam_u32     ("gid",                         OPT_GID),
>         fsparam_u32oct  ("mode",                        OPT_MODE),
> +       fsparam_u32     ("uid",                         OPT_UID),
>         {}
>  };
>
>  struct bpf_mount_opts {
> +       kuid_t uid;
> +       kgid_t gid;
>         umode_t mode;
>  };
>
> @@ -641,6 +654,8 @@ static int bpf_parse_param(struct fs_context *fc, struct fs_parameter *param)
>  {
>         struct bpf_mount_opts *opts = fc->fs_private;
>         struct fs_parse_result result;
> +       kuid_t uid;
> +       kgid_t gid;
>         int opt;
>
>         opt = fs_parse(fc, bpf_fs_parameters, param, &result);
> @@ -662,12 +677,43 @@ static int bpf_parse_param(struct fs_context *fc, struct fs_parameter *param)
>         }
>
>         switch (opt) {
> +       case OPT_UID:
> +               uid = make_kuid(current_user_ns(), result.uint_32);
> +               if (!uid_valid(uid))
> +                       goto bad_value;
> +
> +               /*
> +                * The requested uid must be representable in the
> +                * filesystem's idmapping.
> +                */
> +               if (!kuid_has_mapping(fc->user_ns, uid))
> +                       goto bad_value;
> +
> +               opts->uid = uid;
> +               break;
> +       case OPT_GID:
> +               gid = make_kgid(current_user_ns(), result.uint_32);
> +               if (!gid_valid(gid))
> +                       goto bad_value;
> +
> +               /*
> +                * The requested gid must be representable in the
> +                * filesystem's idmapping.
> +                */
> +               if (!kgid_has_mapping(fc->user_ns, gid))
> +                       goto bad_value;
> +
> +               opts->gid = gid;
> +               break;
>         case OPT_MODE:
>                 opts->mode = result.uint_32 & S_IALLUGO;
>                 break;
>         }
>
>         return 0;
> +
> +bad_value:
> +       return invalfc(fc, "Bad value for '%s'", param->key);
>  }
>
>  struct bpf_preload_ops *bpf_preload_ops;
> @@ -750,6 +796,8 @@ static int bpf_fill_super(struct super_block *sb, struct fs_context *fc)
>         sb->s_op = &bpf_super_ops;
>
>         inode = sb->s_root->d_inode;
> +       inode->i_uid = opts->uid;
> +       inode->i_gid = opts->gid;
>         inode->i_op = &bpf_dir_iops;
>         inode->i_mode &= ~S_IALLUGO;
>         populate_bpffs(sb->s_root);
> --
> 2.43.0.rc2.451.g8631bc7472-goog
>

Alexei Starovoitov Dec. 6, 2023, 6:20 p.m. UTC | #3

On Wed, Dec 6, 2023 at 9:21 AM Andrii Nakryiko
<[e-mail]> wrote:
>
> On Tue, Dec 5, 2023 at 11:36 PM Jie Jiang <[e-mail]> wrote:
> >
> > Parse uid and gid in bpf_parse_param() so that they can be passed in as
> > the `data` parameter when mount() bpffs. This will be useful when we
> > want to control which user/group has the control to the mounted bpffs,
> > otherwise a separate chown() call will be needed.
> >
> > Signed-off-by: Jie Jiang <[e-mail]>
> > ---
> > v1 -> v2: Add additional validation in bpf_parse_param() for if the
> >   requested uid/gid is representable in the fs's idmapping.
> >
> >  kernel/bpf/inode.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 50 insertions(+), 2 deletions(-)
> >
>
> LGTM, but I want to point out that this will conflict with the BPF
> token series ([0]), so depending which one goes in first, the other
> will have to be rebased.

The token series are much bigger, so I applied them first.
Please rebase, resend, and keep acks.

> Acked-by: Andrii Nakryiko <[e-mail]>

Jie Jiang Dec. 7, 2023, 4:32 a.m. UTC | #4

On Thu, Dec 7, 2023 at 3:20 AM Alexei Starovoitov
<[e-mail]> wrote:
> [...]
>
> The token series are much bigger, so I applied them first.
> Please rebase, resend, and keep acks.

I have uploaded v3 to resolve the conflicts and also keep the acks.
Thanks!

Patch

diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index 1aafb2ff2e953..5bc79535d3357 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -599,8 +599,15 @@  EXPORT_SYMBOL(bpf_prog_get_type_path);
  */
 static int bpf_show_options(struct seq_file *m, struct dentry *root)
 {
-	umode_t mode = d_inode(root)->i_mode & S_IALLUGO & ~S_ISVTX;
-
+	struct inode *inode = d_inode(root);
+	umode_t mode = inode->i_mode & S_IALLUGO & ~S_ISVTX;
+
+	if (!uid_eq(inode->i_uid, GLOBAL_ROOT_UID))
+		seq_printf(m, ",uid=%u",
+			   from_kuid_munged(&init_user_ns, inode->i_uid));
+	if (!gid_eq(inode->i_gid, GLOBAL_ROOT_GID))
+		seq_printf(m, ",gid=%u",
+			   from_kgid_munged(&init_user_ns, inode->i_gid));
 	if (mode != S_IRWXUGO)
 		seq_printf(m, ",mode=%o", mode);
 	return 0;
@@ -625,15 +632,21 @@  static const struct super_operations bpf_super_ops = {
 };
 
 enum {
+	OPT_UID,
+	OPT_GID,
 	OPT_MODE,
 };
 
 static const struct fs_parameter_spec bpf_fs_parameters[] = {
+	fsparam_u32	("gid",				OPT_GID),
 	fsparam_u32oct	("mode",			OPT_MODE),
+	fsparam_u32	("uid",				OPT_UID),
 	{}
 };
 
 struct bpf_mount_opts {
+	kuid_t uid;
+	kgid_t gid;
 	umode_t mode;
 };
 
@@ -641,6 +654,8 @@  static int bpf_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
 	struct bpf_mount_opts *opts = fc->fs_private;
 	struct fs_parse_result result;
+	kuid_t uid;
+	kgid_t gid;
 	int opt;
 
 	opt = fs_parse(fc, bpf_fs_parameters, param, &result);
@@ -662,12 +677,43 @@  static int bpf_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	}
 
 	switch (opt) {
+	case OPT_UID:
+		uid = make_kuid(current_user_ns(), result.uint_32);
+		if (!uid_valid(uid))
+			goto bad_value;
+
+		/*
+		 * The requested uid must be representable in the
+		 * filesystem's idmapping.
+		 */
+		if (!kuid_has_mapping(fc->user_ns, uid))
+			goto bad_value;
+
+		opts->uid = uid;
+		break;
+	case OPT_GID:
+		gid = make_kgid(current_user_ns(), result.uint_32);
+		if (!gid_valid(gid))
+			goto bad_value;
+
+		/*
+		 * The requested gid must be representable in the
+		 * filesystem's idmapping.
+		 */
+		if (!kgid_has_mapping(fc->user_ns, gid))
+			goto bad_value;
+
+		opts->gid = gid;
+		break;
 	case OPT_MODE:
 		opts->mode = result.uint_32 & S_IALLUGO;
 		break;
 	}
 
 	return 0;
+
+bad_value:
+	return invalfc(fc, "Bad value for '%s'", param->key);
 }
 
 struct bpf_preload_ops *bpf_preload_ops;
@@ -750,6 +796,8 @@  static int bpf_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_op = &bpf_super_ops;
 
 	inode = sb->s_root->d_inode;
+	inode->i_uid = opts->uid;
+	inode->i_gid = opts->gid;
 	inode->i_op = &bpf_dir_iops;
 	inode->i_mode &= ~S_IALLUGO;
 	populate_bpffs(sb->s_root);