Indeed, this function is badly written. In rewriting, I couldn't find a nice function for reading integers from userspace, and the proc_dointvec stuff didn't seem terribly suitable. So I wrote my own general function, which I can move someplace else if someone likes. Patch is attached. It should not affect correct usage of this file. Thank you for pointing this out. Andrew, can you include this? Matt, can you test? -Corey On Thu, 2005-09-01 at 07:43 +0100, viro@ZenIV.linux.org.uk wrote: > drivers/char/ipmi/ipmi_poweroff.c::proc_write_chassctrl() > a) does sscanf on userland pointer > b) does sscanf on array that is not guaranteed to have NUL in it > c) interprets input in incredibly cretinous way: > if strings doesn't start with a decimal number => as if it was "0". > if it starts with decimal number equal to 0 (e.g. is "-0000splat") - as if > it was "0". > if it starts with decimal number equal to 2 (e.g. is "00002FOAD") - as if > it was "2". > otherwise - -EINVAL. > In any case that doesn't end up with -EINVAL, pretend that entire > buffer had been written. > > (a) and (b) are immediate bugs; (c) is a valid reason for immediate severe > LARTing of the pervert who had done _that_ in a user-visible API. > > Note that API _is_ user-visible, so we can't blindly change it - not without > checking WTF do its users actually write to /proc/ipmi/poweroff_control. > > Could somebody comment on the actual uses of that FPOS? My preference would > be to remove the damn thing completely - it's too ugly to live.