Subject: Re: [PATCH] net: fix /proc/net/snmp as memory corruptor

From: Eric Dumazet <[email-masked]>
Date: Sat, 08 Nov 2008 21:46:36 +0100

David Stevens a écrit :
> Oh, that was me! Thank you, Alexey!
> 
>> This should be fixed as well, or multiple threads reading /proc/net/snmp
>> could get mixed results without proper locking.
> 
> I don't believe locking is an issue here. If the values
> change between the first and second tests, being counters,
> they are still nonzero. If they are different in different
> threads, it reflects an actual change in the counter. So
> I'm not sure what you're talking about here.

If you are not sure what I am talking about, then you should probably
not use static variables at all. I found this fix quite obvious...

> 
> I don't think they should be on the stack (obviously, or
> I wouldn't have written it this way). So, FWIW, I like
> Alexey's fix, which is what the code should've been.

You apparently missed the fact that with your new code, we can have more than
16 different ICMP counters > 0.

thread 1 on CPU 1
-----------------

- fills 16 indexes in static table
- print them. good.

- fills *next* 16 indexes in static table
... preempted by some IRQ or something....

thread 2 on CPU 2
------------------

fills 16 indexes in static table, overwriting
the values that thread 1 was trying to put.
...

thread 1:

print the values of thread 2.
(it will probably prints a copy of its first line)

bang : User application missed some critical information.

> 
> For Alexey's patch:
> 
> Acked-by: David L Stevens <[email-masked]>