| Message ID | 1600398200-8198-1-git-send-email-xiyuyang19@fudan.edu.cn |
|---|---|
| State | New |
| Headers | show |
| Series | batman-adv: Fix orig node refcnt leak when creating neigh node | expand |
On Friday, 18 September 2020 05:03:19 CEST Xiyu Yang wrote: > batadv_neigh_node_create() is used to create a neigh node object, whose > fields will be initialized with the specific object. When a new > reference of the specific object is created during the initialization, > its refcount should be increased. > > However, when "neigh_node" object initializes its orig_node field with > the "orig_node" object, the function forgets to hold the refcount of the > "orig_node", causing a potential refcount leak and use-after-free issue > for the reason that the object can be freed in other places. > > Fix this issue by increasing the refcount of orig_node object during the > initialization and adding corresponding batadv_orig_node_put() in > batadv_neigh_node_release(). I will most likely not add this patch because I have concerns that this would need an active garbage collector to fix the reference counter loop. Please check batadv_neigh_node::orig_node (whose reference counter you've just incremented) and batadv_orig_node::neigh_list (with batadv_neigh_node). And at the same time the batadv_neigh_node_release and batadv_orig_node_release. So the originator will only free the reference (and thus potentially call batadv_neigh_node_release) when its own reference counter is zero. But it cannot become zero because the neigh_node is holding a reference to this originator. Kind regards, Sven
diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c index 805d8969bdfb..d6c2296f8e35 100644 --- a/net/batman-adv/originator.c +++ b/net/batman-adv/originator.c @@ -306,6 +306,8 @@ static void batadv_neigh_node_release(struct kref *ref) batadv_hardif_put(neigh_node->if_incoming); + batadv_orig_node_put(neigh_node->orig_node); + kfree_rcu(neigh_node, rcu); } @@ -685,6 +687,7 @@ batadv_neigh_node_create(struct batadv_orig_node *orig_node, kref_get(&hard_iface->refcount); ether_addr_copy(neigh_node->addr, neigh_addr); neigh_node->if_incoming = hard_iface; + kref_get(&orig_node->refcount); neigh_node->orig_node = orig_node; neigh_node->last_seen = jiffies;