Re: [PATCH] netrom: Prevent race conditions between multiple add route
From: Dan Carpenter <hidden>
Date: 2025-10-20 10:10:15
Also in:
linux-hams, lkml
On Mon, Oct 20, 2025 at 04:13:59PM +0800, Lizhi Xu wrote:
The root cause of the problem is that multiple different tasks initiate NETROM_NODE commands to add new routes, there is no lock between them to protect the same nr_neigh. Task0 may add the nr_neigh.refcount value of 1 on Task1 to routes[2]. When Task3 executes nr_neigh_put(nr_node->routes[2].neighbour), it will
s/Task3/Task1/
release the neighbour because its refcount value is 1.
The refcount would be 2 and then drop to zero. Both nr_neigh_put() and nr_remove_neigh() drop the refcount.
In this case, the following situation causes a UAF: Task0 Task1 ===== ===== nr_add_node() nr_neigh_get_dev() nr_add_node() nr_node->routes[2].neighbour->count--
Does this line really matter in terms of the use after free?
nr_neigh_put(nr_node->routes[2].neighbour); nr_remove_neigh(nr_node->routes[2].neighbour) nr_node->routes[2].neighbour = nr_neigh nr_neigh_hold(nr_neigh);
This chart is confusing. It says that that the nr_neigh_hold() is the use after free. But we called nr_remove_neigh(nr_node->routes[2].neighbour) before we assigned nr_node->routes[2].neighbour = nr_neigh... The sysbot report says that the free happens on: r_neigh_put(nr_node->routes[2].neighbour); and the use after free happens on the next line: if (nr_node->routes[2].neighbour->count == 0 && !nr_node->routes[2].neighbour->locked) Which does suggest that somewhere the refcount is 1 when it should be at least 2... It could be that two threads call nr_neigh_put() at basically the same time, but that doesn't make sense either because we're holding the nr_node_lock(nr_node)... regards, dan carpenter