Messages in this thread |  | | | Date | Thu, 31 Jan 2008 23:36:26 -0800 | | From | Andrew Morton <> | | Subject | Re: [PATCH 2.6.24] Add new string functions strict_strto* and convert kernel params to use them, take 2 |
| |
On Fri, 01 Feb 2008 06:11:43 +0800 Yi Yang <yi.y.yang@intel.com> wrote:
> This patch is a resend, it changes previous name "real_" to "strict_" > according to Randy Dunlap's feedback. Please consider to apply. thanks. > > 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/uul, 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 > succesive invalid char and converts prefix part to a numeric although > input is acctually invalid. > > This patch tries to fix the aforementioned issues and implements strict_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: > > write error: Invalid argument > > Any modules which export numeric sysfs node can use strict_strtox instead of > simple_strtox to reject any invalid input. > > 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 /]# >
Oh dear. So if someone has an existing script which does
echo 0x1000g > /sys/module/e1000/parameters/copybreak we just broke their setup.
This is what happens when we screw up kernel interfaces: we should remain bug-for-bug compatible with earlier releases. argh, we suck :(
I'm inclined to merge it anyway - everyone already knows we suck, and not many people will be affected and they suck too.
|  |