Messages in this thread | | | Date | Sun, 30 Aug 2015 00:44:41 +0300 | From | Alexey Dobriyan <> | Subject | Re: [PATCH v2] devpts: allow mounting with uid/gid of uint32_t |
| |
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);
| |