Re: [PATCH 07/14] fuse: Convert to new uid/gid option parsing helpers

[Christian Brauner <brauner@kernel.org>]

I think you accidently Cced the wrong Miklos. :)

On Thu, Jun 27, 2024 at 07:33:43PM GMT, Eric Sandeen wrote:
> Convert to new uid/gid option parsing helpers
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  fs/fuse/inode.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 99e44ea7d875..1ac528bcdb3c 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -740,8 +740,8 @@ static const struct fs_parameter_spec fuse_fs_parameters[] = {
>  	fsparam_string	("source",		OPT_SOURCE),
>  	fsparam_u32	("fd",			OPT_FD),
>  	fsparam_u32oct	("rootmode",		OPT_ROOTMODE),
> -	fsparam_u32	("user_id",		OPT_USER_ID),
> -	fsparam_u32	("group_id",		OPT_GROUP_ID),
> +	fsparam_uid	("user_id",		OPT_USER_ID),
> +	fsparam_gid	("group_id",		OPT_GROUP_ID),
>  	fsparam_flag	("default_permissions",	OPT_DEFAULT_PERMISSIONS),
>  	fsparam_flag	("allow_other",		OPT_ALLOW_OTHER),
>  	fsparam_u32	("max_read",		OPT_MAX_READ),
> @@ -799,16 +799,12 @@ static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
>  		break;
>  
>  	case OPT_USER_ID:
> -		ctx->user_id = make_kuid(fsc->user_ns, result.uint_32);
> -		if (!uid_valid(ctx->user_id))
> -			return invalfc(fsc, "Invalid user_id");
> +		ctx->user_id = result.uid;

So fsc->user_ns will record the namespaces at fsopen() time which can be
different from the namespace used at fsconfig() time. This was done when
fuse was ported to the new mount api.

It has the same potential issues that Seth pointed out so I think your
patch is correct. But I also think we might need the same verification
that tmpfs is doing to verify that the uid/gid we're using can actually
be represented in the fsc->user_ns.

So maybe there should be a separate patch that converts fuse to rely on
make_k*id(current_user_ns()) + k*id_has_mapping() and then these patches
on top?