On Fri, 01 Feb 2008 06:11:43 +0800 Yi Yang <[EMAIL PROTECTED]> 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: > > [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] /]# > 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. -- 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/