Subject: [PATCH] net: net_assign_generic() fix

From: Eric Dumazet <[email-masked]>
Date: Tue, 28 Jul 2009 14:36:15 +0200

Eric Dumazet a écrit :
> Pavel Emelyanov a écrit :
>> Eric Dumazet wrote:
>>> Igor M Podlesny a écrit :
>>>> [...]
>>>>> Could have been a problem in net core, perhaps.
>>>>>
>>>>> Below is a ppp fix from 2.6.31, but it seems unlikely to fix your problem.
>>>>>
>>>>> It would help if we could see that trace, please.  A digital photo
>>>>> would suit.
>>>> 	Here it is:
>>>>
>>>> 		http://bugzilla.kernel.org/attachment.cgi?id=22516
>>>>
>>>> 	(It's 2.6.30.3)
>>>> 	
>>> Looking at this, I believe net_assign_generic() is not safe.
>>>
>>> Two cpus could try to expand/update the array at same time, one update could be lost.
>>>
>>> register_pernet_gen_device() has a mutex to guard against concurrent
>>> calls, but net_assign_generic() has no locking at all.
>>>
>>> I doubt this is the reason of the crash, still worth to mention it...
>>>
>>> [PATCH] net: net_assign_generic() is not SMP safe
>>>
>>> Two cpus could try to expand/update the array at same time, one update
>>> could be lost during the copy of old array.
>> How can this happen? The array is updated only during ->init routines
>> of the pernet_operations, which are called from under the net_mutex.
>>
>> Do I miss anything?
>>
> 
> Oops, I missed the obvious "BUG_ON(!mutex_is_locked(&net_mutex));"
> 
> Sorry for the noise and untested patch as well :)
>

Hmm...

Real bug may be fixed by followed patch ? (yet untested, sorry...)

[PATCH] net: net_assign_generic() fix 

memcpy() should take into account size of pointers,
not only number of pointers to copy.

Signed-off-by: Eric Dumazet <[email-masked]>
---
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index b7292a2..1972830 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -488,7 +488,7 @@ int net_assign_generic(struct net *net, int id, void *data)
 	 */
 
 	ng->len = id;
-	memcpy(&ng->ptr, &old_ng->ptr, old_ng->len);
+	memcpy(&ng->ptr, &old_ng->ptr, old_ng->len * sizeof(void*));
 
 	rcu_assign_pointer(net->gen, ng);
 	call_rcu(&old_ng->rcu, net_generic_release);