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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to