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
signature.asc
Description: OpenPGP digital signature
