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: > > [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak > 4096 > [EMAIL PROTECTED] /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak > [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak > 4096 > [EMAIL PROTECTED] /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak > [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak > 4096 > [EMAIL PROTECTED] /]# echo 0x1000gggggggg > > /sys/module/e1000/parameters/copybreak > [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak > 4096 > [EMAIL PROTECTED] /]# echo 010000 > /sys/module/e1000/parameters/copybreak > [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak > 4096 > [EMAIL PROTECTED] /]# echo 0100008 > /sys/module/e1000/parameters/copybreak > [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak > 4096 > [EMAIL PROTECTED] /]# echo 010000aaaaa > > /sys/module/e1000/parameters/copybreak > [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak > 4096 > [EMAIL PROTECTED] /]# > > > After applying this patch: > > [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak > 4096 > [EMAIL PROTECTED] /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak > [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak > 4096 > [EMAIL PROTECTED] /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak > -bash: echo: write error: Invalid argument > [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak > 4096 > [EMAIL PROTECTED] /]# echo 0x1000gggggggg > > /sys/module/e1000/parameters/copybreak > -bash: echo: write error: Invalid argument > [EMAIL PROTECTED] /]# echo 010000 > /sys/module/e1000/parameters/copybreak > [EMAIL PROTECTED] /]# echo 0100008 > /sys/module/e1000/parameters/copybreak > -bash: echo: write error: Invalid argument > [EMAIL PROTECTED] /]# echo 010000aaaaa > > /sys/module/e1000/parameters/copybreak > -bash: echo: write error: Invalid argument > [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak > 4096 > [EMAIL PROTECTED] /]# echo -n 4096 > /sys/module/e1000/parameters/copybreak > [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak > 4096 > [EMAIL PROTECTED] /]# > > > Signed-off-by: Yi Yang <[EMAIL PROTECTED]> > --- > 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 [EMAIL PROTECTED] > 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/