Date: Sun, 30 Aug 2015 00:44:41 +0300
From: Alexey Dobriyan <adobriyan@gmail.com>
To: Dongsu Park <dpark@posteo.net>
Cc: Peter Hurley <peter@hurleysoftware.com>, Al Viro <viro@zeniv.linux.org.uk>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        Josh Triplett <josh@joshtriplett.org>,
        David Howells <dhowells@redhat.com>, Alban Crequy <alban@endocode.com>
Subject: Re: [PATCH v2] devpts: allow mounting with uid/gid of uint32_t
Message-ID: <20150829214441.GA32321@p183.telecom.by>
References: <882a878038efb5fed381be5d4817ff44d90703d5.1439904209.git.dpark@posteo.net>
 <692839ff7158dbb96dd20ce8e36c13f85fa64fd7.1439910753.git.dpark@posteo.net>
 <55E0B786.70101@hurleysoftware.com>
 <20150829104304.GA2841@posteo.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150829104304.GA2841@posteo.de>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3288
Lines: 119

On Sat, Aug 29, 2015 at 12:43:04PM +0200, Dongsu Park wrote:
> > How about adding a for-purpose string-to-uid/gid function, rather than
> > open-coding?

>  		case Opt_uid:
> -			if (match_int(&args[0], &option))
> -				return -EINVAL;
> -			uid = make_kuid(current_user_ns(), option);
> -			if (!uid_valid(uid))
> -				return -EINVAL;
> +			rc = kstrtouid(args[0].from, &uid);
> +			if (rc)
> +				return rc;
> +
>  			opts->uid = uid;

if kstrtouid is doing all the work, value should be put directly into
opts->uid avoiding temporary variables.

>  		case Opt_gid:
> -			if (match_int(&args[0], &option))
> -				return -EINVAL;
> -			gid = make_kgid(current_user_ns(), option);
> -			if (!gid_valid(gid))
> -				return -EINVAL;
> +			rc = kstrtogid(args[0].from, &gid);
> +			if (rc)
> +				return rc;

ditto

> +
>  			opts->gid = gid;
>  			opts->setgid = 1;
>  			break;

> --- a/include/linux/parse-integer.h
> +++ b/include/linux/parse-integer.h
> @@ -2,6 +2,7 @@
>  #define _PARSE_INTEGER_H
>  #include <linux/compiler.h>
>  #include <linux/types.h>
> +#include <linux/uidgid.h>

no, just put the prototypes into uidgid.h
This header is supposed to be small and self contained.

> @@ -155,6 +156,9 @@ static inline int __must_check kstrtos8(const char *s, unsigned int base, s8 *re
>  	return parse_integer(s, base | PARSE_INTEGER_NEWLINE, res);
>  }
>  
> +int __must_check kstrtouid(const char *uidstr, kuid_t *kuid);
> +int __must_check kstrtogid(const char *gidstr, kgid_t *kgid);
> +
>  int __must_check kstrtoull_from_user(const char __user *s, size_t count, unsigned int base, unsigned long long *res);
>  int __must_check kstrtoll_from_user(const char __user *s, size_t count, unsigned int base, long long *res);
>  int __must_check kstrtoul_from_user(const char __user *s, size_t count, unsigned int base, unsigned long *res);

> --- a/lib/kstrtox.c
> +++ b/lib/kstrtox.c
> @@ -17,6 +17,8 @@
>  #include <linux/math64.h>
>  #include <linux/export.h>
>  #include <linux/types.h>
> +#include <linux/sched.h>
> +#include <linux/cred.h>

This is pretty ridiculous for what is supposed to be bunch  of integer
conversion routines. sched.h? come on!

> @@ -81,6 +83,44 @@ int f(const char __user *s, size_t count, unsigned int base, type *res)	\
>  }									\
>  EXPORT_SYMBOL(f)
>  
> +int kstrtouid(const char *uidstr, kuid_t *kuid)
> +{
> +	uid_t uidval;
> +	kuid_t kuidtmp;
> +
> +	int rc = kstrtouint(uidstr, 0, &uidval);
> +
> +	if (rc)
> +		return rc;

and please follow coding style.

> +
> +	kuidtmp = make_kuid(current_user_ns(), uidval);
> +	if (!uid_valid(kuidtmp))
> +		return -EINVAL;
> +
> +	*kuid = kuidtmp;
> +	return 0;
> +}
> +EXPORT_SYMBOL(kstrtouid);
> +
> +int kstrtogid(const char *gidstr, kgid_t *kgid)
> +{
> +	gid_t gidval;
> +	kgid_t kgidtmp;
> +
> +	int rc = kstrtouint(gidstr, 0, &gidval);
> +
> +	if (rc)
> +		return rc;
> +
> +	kgidtmp = make_kgid(current_user_ns(), gidval);
> +	if (!gid_valid(kgidtmp))
> +		return -EINVAL;
> +
> +	*kgid = kgidtmp;
> +	return 0;
> +}
> +EXPORT_SYMBOL(kstrtogid);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/