Re: [PATCH 2.6.24] Add new string functions real_strtoul and changekernel params to use them

From: Randy Dunlap
Date: Thu Jan 31 2008 - 12:06:00 EST

Next message: Kiyoshi Ueda: "Re: kernel BUG at ide-cd.c:1726 in 2.6.24-03863-g0ba6c33 &&-g8561b089"
Previous message: Greg KH: "Re: [PATCH] kobject: Always build in kernel/ksysfs.o."
In reply to: Yi Yang: "[PATCH 2.6.24] Add new string functions real_strtoul and changekernel params to use them"
Next in thread: Yi Yang: "Re: [PATCH 2.6.24] Add new string functions real_strtoul andchange kernel params to use them"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Thu, 31 Jan 2008 09:18:22 +0800 Yi Yang wrote:

> Currently, for every sysfs node, the callers will be responsible for
> implementing store operation, so many many callers are doing duplicate
> things to validate input, they have the same mistakes because they are
> calling simple_strtol/ul/ll/ull, especially for module params, they are
> just numeric, but you can echo such values as 0x1234xxx, 07777888 and
> 1234aaa, for these cases, module params store operation just ignores
> successive invalid char and converts prefix part to a numeric although
> input is actually invalid.
>
> This patch tries to fix the aforementioned issues and implements real_strtox
> serial functions, kernel/params.c uses them to strictly validate input,
> so module params will reject such values as 0x1234xxxx and returns an error:

How about a prefix of safe_ or strict_ or something other than real_?
real_ sounds too much like a real number function string parser.

> write error: Invalid argument
>
> Any modules which export numeric sysfs node can use real_strtox instead of
> simple_strtox to reject any invalid input. Please consider to merge to -mm tree
> in order to test.
>
> Here are some test results:
>
> Before applying this patch:
>
> [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
> 4096
> [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
> [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
> 4096
> [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
> [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
> 4096
> [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak
> [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
> 4096
> [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak
> [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
> 4096
> [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak
> [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
> 4096
> [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak
> [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
> 4096
> [root@yangyi-dev /]#
>
>
> After applying this patch:
>
> [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
> 4096
> [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
> [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
> 4096
> [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
> -bash: echo: write error: Invalid argument
> [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
> 4096
> [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak
> -bash: echo: write error: Invalid argument
> [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak
> [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak
> -bash: echo: write error: Invalid argument
> [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak
> -bash: echo: write error: Invalid argument
> [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
> 4096
> [root@yangyi-dev /]# echo -n 4096 > /sys/module/e1000/parameters/copybreak
> [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
> 4096
> [root@yangyi-dev /]#
>
>
> Signed-off-by: Yi Yang <yi.y.yang@xxxxxxxxx>
> ---
> include/linux/kernel.h | 4 ++++
> kernel/params.c | 20 ++++++++++----------
> lib/vsprintf.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 60 insertions(+), 10 deletions(-)
>
> --- a/include/linux/kernel.h 2008-01-31 00:41:46.000000000 +0800
> +++ b/include/linux/kernel.h 2008-01-31 01:12:33.000000000 +0800
> @@ -141,6 +141,10 @@ extern unsigned long simple_strtoul(cons
> extern long simple_strtol(const char *,char **,unsigned int);
> extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
> extern long long simple_strtoll(const char *,char **,unsigned int);
> +extern int real_strtoul(const char *, unsigned int, unsigned long *);
> +extern int real_strtol(const char *, unsigned int, long *);
> +extern int real_strtoull(const char *, unsigned int, unsigned long long *);
> +extern int real_strtoll(const char *, unsigned int, long long *);
> extern int sprintf(char * buf, const char * fmt, ...)
> __attribute__ ((format (printf, 2, 3)));
> extern int vsprintf(char *buf, const char *, va_list)
> --- a/lib/vsprintf.c 2008-01-30 08:13:03.000000000 +0800
> +++ b/lib/vsprintf.c 2008-01-31 05:19:31.000000000 +0800
> @@ -126,6 +126,52 @@ long long simple_strtoll(const char *cp,
> return simple_strtoull(cp,endp,base);
> }
>
> +#define define_real_strtoux(type, valtype) \
> +int real_strtou##type(const char *cp, unsigned int base, valtype *res) \
> +{ \
> + char *tail; \
> + valtype val; \
> + size_t len; \
> + \
> + *res = 0; \
> + len = strlen(cp); \
> + if (len == 0) \
> + return -EINVAL; \
> + \
> + val = simple_strtoul(cp, &tail, base); \
> + if ((*tail == '\0') || \
> + (len == (size_t)(tail - cp) + 1) && (*tail == '\n')) { \
> + *res = val; \
> + return 0; \
> + } \
> + \
> + return -EINVAL; \
> +} \
> +
> +#define define_real_strtox(type, valtype) \
> +int real_strto##type(const char *cp, unsigned int base, valtype *res) \
> +{ \
> + int ret; \
> + if (*cp == '-') { \
> + ret = real_strtou##type(cp+1, base, res); \
> + if (ret != 0) \
> + *res = -(*res); \
> + } else \
> + ret = real_strtou##type(cp+1, base, res); \
> + \
> + return ret; \
> +} \
> +
> +define_real_strtoux(l, unsigned long)
> +define_real_strtox(l, long)
> +define_real_strtoux(ll, unsigned long long)
> +define_real_strtox(ll, long long)
> +
> +EXPORT_SYMBOL(real_strtoul);
> +EXPORT_SYMBOL(real_strtol);
> +EXPORT_SYMBOL(real_strtoll);
> +EXPORT_SYMBOL(real_strtoull);
> +
> static int skip_atoi(const char **s)
> {
> int i=0;
> --- a/kernel/params.c 2008-01-31 00:44:44.000000000 +0800
> +++ b/kernel/params.c 2008-01-31 01:11:51.000000000 +0800
> @@ -180,12 +180,12 @@ int parse_args(const char *name,
> #define STANDARD_PARAM_DEF(name, type, format, tmptype, strtolfn) \
> int param_set_##name(const char *val, struct kernel_param *kp) \
> { \
> - char *endp; \
> tmptype l; \
> + int ret; \
> \
> if (!val) return -EINVAL; \
> - l = strtolfn(val, &endp, 0); \
> - if (endp == val || ((type)l != l)) \
> + ret = strtolfn(val, 0, &l); \
> + if (ret == -EINVAL || ((type)l != l)) \
> return -EINVAL; \
> *((type *)kp->arg) = l; \
> return 0; \
> @@ -195,13 +195,13 @@ int parse_args(const char *name,
> return sprintf(buffer, format, *((type *)kp->arg)); \
> }
>
> -STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, simple_strtoul);
> -STANDARD_PARAM_DEF(short, short, "%hi", long, simple_strtol);
> -STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, simple_strtoul);
> -STANDARD_PARAM_DEF(int, int, "%i", long, simple_strtol);
> -STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, simple_strtoul);
> -STANDARD_PARAM_DEF(long, long, "%li", long, simple_strtol);
> -STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, simple_strtoul);
> +STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, real_strtoul);
> +STANDARD_PARAM_DEF(short, short, "%hi", long, real_strtol);
> +STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, real_strtoul);
> +STANDARD_PARAM_DEF(int, int, "%i", long, real_strtol);
> +STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, real_strtoul);
> +STANDARD_PARAM_DEF(long, long, "%li", long, real_strtol);
> +STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, real_strtoul);
>
> int param_set_charp(const char *val, struct kernel_param *kp)
> {
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

---
~Randy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Kiyoshi Ueda: "Re: kernel BUG at ide-cd.c:1726 in 2.6.24-03863-g0ba6c33 &&-g8561b089"
Previous message: Greg KH: "Re: [PATCH] kobject: Always build in kernel/ksysfs.o."
In reply to: Yi Yang: "[PATCH 2.6.24] Add new string functions real_strtoul and changekernel params to use them"
Next in thread: Yi Yang: "Re: [PATCH 2.6.24] Add new string functions real_strtoul andchange kernel params to use them"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]