On Wed, Apr 10, 2019 at 8:46 PM Kees Cook <keescook@xxxxxxxxxxxx> wrote: > > On Mon, Apr 8, 2019 at 3:09 PM Matteo Croce <mcroce@xxxxxxxxxx> wrote: > > > > Use the shared variables for range check, instead of declaring a local one > > in every source file. > > I was expecting this to be a tree-wide change for all the cases found > by patch 1's "git grep". > Hi Kees, I have already the whole patch ready, but I was frightened by the output of get_maintainer.pl, so I decided to split the patch into small pieces and send the first one. Patches for /proc/sys/net and drivers/ are pretty big, and can be merged after the 1/2 inclusion. > Slight change to the grep for higher accuracy: > > $ git grep -E '\.extra[12].*&(zero|one|int_max)\b' |wc -l > 245 > Right, my regexp wrongly matches also one_hundred, one_jiffy, etc. Anywqay, I did the changes by hand, so apart the commit message, the content should be safe. > Only 31 sources: > $ git grep -E '\.extra[12].*&(zero|one|int_max)\b' | cut -d: -f1 | > sort -u > /tmp/list.txt > $ wc -l /tmp/list.txt > 31 > > One thing I wonder about is if any of these cases depend on the extra > variable being non-const (many of these are just "static int"). > > $ egrep -H '\b(zero|one|int_max)\b.*=' $(cat /tmp/list.txt) | grep -v static > > Looks like none, so it'd be safe. How about doing this tree-wide for > all 31 cases? (Coccinelle might be able to help.) > It could be true for other sysctl values like xpc_disengage_max_timelimit or fscache_op_wq, but it's very unlikely that someone writes, for example, 5 into a variable named "zero". If it does, it most likely a bug, so const is our friend. Regards, -- Matteo Croce per aspera ad upstream
