Re: [B.A.T.M.A.N.] [PATCH-maint] batman-adv: fix potential orig_node reference leak

[Antonio Quartulli <antonio@meshcoding.com>]

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

On 11/02/14 18:40, Marek Lindner wrote:
> On Tuesday 11 February 2014 18:34:10 Antonio Quartulli wrote:
>> true, but since the orig_node has not been returned yet there is no
>> other component in batman-adv which is using it.
>>
>> Otherwise, we may want to define and invoke a free_now() version of this
>> function (like we have done for other objects). But I think kfree() is
>> safe here.
> 
> It may be safe now but will surely be forgotten later. A guaranteed source for 
> trouble. That is why we have cleanup routines for everything.
> 

True, in particular because we (as bat_iv_ogm.c) do not know what
batadv_orig_node_new() has allocated - a kfree() would be fine if the
object was allocated with a plain kmalloc(), but this is not the case.

By the way we have a bug here:
if we jump to

 256 free_bcast_own:
 257         kfree(orig_node->bat_iv.bcast_own);

bcast_own gets free'd but not assigned to NULL. Later
batadv_orig_node_free_rcu() (scheduled by batadv_orig_node_free_ref())
will call batadv_iv_ogm_orig_free() that will try to kfree() bcast_own
again (line 98 in bat_iv_ogm.c), thus leading to a double free. no?


Cheers,


-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]