Re: [PATCH v2] devpts: allow mounting with uid/gid of uint32_t

[Peter Hurley <peter@hurleysoftware.com>]

On 08/18/2015 11:18 AM, Dongsu Park wrote:
> To allow devpts to be mounted with options of uid/gid of uint32_t,
> use kstrtouint() instead of match_int(). Doing that, mounting devpts
> with uid or gid > (2^31 - 1) will work as expected, e.g.:
> 
>  # mount -t devpts devpts /tmp/devptsdir -o \
>    newinstance,ptmxmode=0666,mode=620,uid=3598450688,gid=3598450693
> 
> It was originally by reported on systemd github issues:
> https://github.com/systemd/systemd/issues/956
> 
> from v1: fix patch format correctly
> 
> Reported-by: Alban Crequy <alban@endocode.com>
> Signed-off-by: Dongsu Park <dpark@posteo.net>
> ---
>  fs/devpts/inode.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
> index c35ffdc12bba..49272fae40a7 100644
> --- a/fs/devpts/inode.c
> +++ b/fs/devpts/inode.c
> @@ -188,23 +188,35 @@ static int parse_mount_options(char *data, int op, struct pts_mount_opts *opts)
>  		token = match_token(p, tokens, args);
>  		switch (token) {
>  		case Opt_uid:
> -			if (match_int(&args[0], &option))

match_int() => make_kuid/kgid is a widespread pattern in filesystems
for handling uid/gid mount parameters.

How about adding a for-purpose string-to-uid/gid function, rather than
open-coding?

Regards,
Peter Hurley


> +		{
> +			char *uidstr = args[0].from;
> +			uid_t uidval;
> +			int rc = kstrtouint(uidstr, 0, &uidval);
> +
> +			if (rc)
>  				return -EINVAL;
> -			uid = make_kuid(current_user_ns(), option);
> +			uid = make_kuid(current_user_ns(), uidval);
>  			if (!uid_valid(uid))
>  				return -EINVAL;
>  			opts->uid = uid;
>  			opts->setuid = 1;
>  			break;
> +		}
>  		case Opt_gid:
> -			if (match_int(&args[0], &option))
> +		{
> +			char *gidstr = args[0].from;
> +			gid_t gidval;
> +			int rc = kstrtouint(gidstr, 0, &gidval);
> +
> +			if (rc)
>  				return -EINVAL;
> -			gid = make_kgid(current_user_ns(), option);
> +			gid = make_kgid(current_user_ns(), gidval);
>  			if (!gid_valid(gid))
>  				return -EINVAL;
>  			opts->gid = gid;
>  			opts->setgid = 1;
>  			break;
> +		}
>  		case Opt_mode:
>  			if (match_octal(&args[0], &option))
>  				return -EINVAL;
>