net/netrom/nr_route.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
The root cause of the problem is that multiple different tasks initiate
SIOCADDRT & NETROM_NODE commands to add new routes, there is no lock
between them to protect the same nr_neigh.
Task0 can add the nr_neigh.refcount value of 1 on Task1 to routes[2].
When Task2 executes nr_neigh_put(nr_node->routes[2].neighbour), it will
release the neighbour because its refcount value is 1.
In this case, the following situation causes a UAF on Task2:
Task0 Task1 Task2
===== ===== =====
nr_add_node()
nr_neigh_get_dev() nr_add_node()
nr_node_lock()
nr_node->routes[2].neighbour->count--
nr_neigh_put(nr_node->routes[2].neighbour);
nr_remove_neigh(nr_node->routes[2].neighbour)
nr_node_unlock()
nr_node_lock()
nr_node->routes[2].neighbour = nr_neigh
nr_neigh_hold(nr_neigh); nr_add_node()
nr_neigh_put()
if (nr_node->routes[2].neighbour->count
Description of the UAF triggering process:
First, Task 0 executes nr_neigh_get_dev() to set neighbor refcount to 3.
Then, Task 1 puts the same neighbor from its routes[2] and executes
nr_remove_neigh() because the count is 0. After these two operations,
the neighbor's refcount becomes 1. Then, Task 0 acquires the nr node
lock and writes it to its routes[2].neighbour.
Finally, Task 2 executes nr_neigh_put(nr_node->routes[2].neighbour) to
release the neighbor. The subsequent execution of the neighbor->count
check triggers a UAF.
Filter out neighbors with a refcount of 1 to avoid unsafe conditions.
syzbot reported:
BUG: KASAN: slab-use-after-free in nr_add_node+0x25db/0x2c00 net/netrom/nr_route.c:248
Read of size 4 at addr ffff888051e6e9b0 by task syz.1.2539/8741
Call Trace:
<TASK>
nr_add_node+0x25db/0x2c00 net/netrom/nr_route.c:248
Reported-by: syzbot+2860e75836a08b172755@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=2860e75836a08b172755
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
V1 -> V2: update comments for cause uaf
V2 -> V3: sync neighbor operations in ioctl and route frame, update comments
V3 -> V4: Preventing the use of neighbors with a reference count of 1
net/netrom/nr_route.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/netrom/nr_route.c b/net/netrom/nr_route.c
index b94cb2ffbaf8..1ef2743a5ec0 100644
--- a/net/netrom/nr_route.c
+++ b/net/netrom/nr_route.c
@@ -100,7 +100,7 @@ static int __must_check nr_add_node(ax25_address *nr, const char *mnemonic,
{
struct nr_node *nr_node;
struct nr_neigh *nr_neigh;
- int i, found;
+ int i, found, ret = 0;
struct net_device *odev;
if ((odev=nr_dev_get(nr)) != NULL) { /* Can't add routes to ourself */
@@ -212,6 +212,10 @@ static int __must_check nr_add_node(ax25_address *nr, const char *mnemonic,
return 0;
}
nr_node_lock(nr_node);
+ if (refcount_read(&nr_neigh->refcount) == 1) {
+ ret = -EINVAL;
+ goto out;
+ }
if (quality != 0)
strscpy(nr_node->mnemonic, mnemonic);
@@ -279,10 +283,11 @@ static int __must_check nr_add_node(ax25_address *nr, const char *mnemonic,
}
}
+out:
nr_neigh_put(nr_neigh);
nr_node_unlock(nr_node);
nr_node_put(nr_node);
- return 0;
+ return ret;
}
static void nr_remove_node_locked(struct nr_node *nr_node)
--
2.43.0On 10/23/25 3:50 PM, Lizhi Xu wrote: > The root cause of the problem is that multiple different tasks initiate > SIOCADDRT & NETROM_NODE commands to add new routes, there is no lock > between them to protect the same nr_neigh. > > Task0 can add the nr_neigh.refcount value of 1 on Task1 to routes[2]. > When Task2 executes nr_neigh_put(nr_node->routes[2].neighbour), it will > release the neighbour because its refcount value is 1. > > In this case, the following situation causes a UAF on Task2: > > Task0 Task1 Task2 > ===== ===== ===== > nr_add_node() > nr_neigh_get_dev() nr_add_node() > nr_node_lock() > nr_node->routes[2].neighbour->count-- > nr_neigh_put(nr_node->routes[2].neighbour); > nr_remove_neigh(nr_node->routes[2].neighbour) > nr_node_unlock() > nr_node_lock() > nr_node->routes[2].neighbour = nr_neigh > nr_neigh_hold(nr_neigh); nr_add_node() > nr_neigh_put() > if (nr_node->routes[2].neighbour->count > Description of the UAF triggering process: > First, Task 0 executes nr_neigh_get_dev() to set neighbor refcount to 3. > Then, Task 1 puts the same neighbor from its routes[2] and executes > nr_remove_neigh() because the count is 0. After these two operations, > the neighbor's refcount becomes 1. Then, Task 0 acquires the nr node > lock and writes it to its routes[2].neighbour. > Finally, Task 2 executes nr_neigh_put(nr_node->routes[2].neighbour) to > release the neighbor. The subsequent execution of the neighbor->count > check triggers a UAF. I looked at the code quite a bit and I think this could possibly avoid the above mentioned race, but this whole area looks quite confusing to me. I think it would be helpful if you could better describe the relevant scenario starting from the initial setup (no nodes, no neighs). Thanks, Paolo
On Tue, 28 Oct 2025 15:13:37 +0100, Paolo Abeni wrote: > > The root cause of the problem is that multiple different tasks initiate > > SIOCADDRT & NETROM_NODE commands to add new routes, there is no lock > > between them to protect the same nr_neigh. > > > > Task0 can add the nr_neigh.refcount value of 1 on Task1 to routes[2]. > > When Task2 executes nr_neigh_put(nr_node->routes[2].neighbour), it will > > release the neighbour because its refcount value is 1. > > > > In this case, the following situation causes a UAF on Task2: > > > > Task0 Task1 Task2 > > ===== ===== ===== > > nr_add_node() > > nr_neigh_get_dev() nr_add_node() > > nr_node_lock() > > nr_node->routes[2].neighbour->count-- > > nr_neigh_put(nr_node->routes[2].neighbour); > > nr_remove_neigh(nr_node->routes[2].neighbour) > > nr_node_unlock() > > nr_node_lock() > > nr_node->routes[2].neighbour = nr_neigh > > nr_neigh_hold(nr_neigh); nr_add_node() > > nr_neigh_put() > > if (nr_node->routes[2].neighbour->count > > Description of the UAF triggering process: > > First, Task 0 executes nr_neigh_get_dev() to set neighbor refcount to 3. > > Then, Task 1 puts the same neighbor from its routes[2] and executes > > nr_remove_neigh() because the count is 0. After these two operations, > > the neighbor's refcount becomes 1. Then, Task 0 acquires the nr node > > lock and writes it to its routes[2].neighbour. > > Finally, Task 2 executes nr_neigh_put(nr_node->routes[2].neighbour) to > > release the neighbor. The subsequent execution of the neighbor->count > > check triggers a UAF. > > I looked at the code quite a bit and I think this could possibly avoid > the above mentioned race, but this whole area looks quite confusing to me. > > I think it would be helpful if you could better describe the relevant > scenario starting from the initial setup (no nodes, no neighs). OK. Let me fill in the origin of neigh. Task3 ===== nr_add_node() [146]if ((nr_neigh = kmalloc(sizeof(*nr_neigh), GFP_ATOMIC)) == NULL) [253]nr_node->routes[2].neighbour = nr_neigh; [255]nr_neigh_hold(nr_neigh); [256]nr_neigh->count++; neigh is created on line 146 in nr_add_node(), and added to node on lines 253-256. It occurs before all Task0, Task1, and Task2. Note: 1. [x], x is line number. 2. During my debugging process, I didn't pay attention to where the node was created, and I apologize that I cannot provide the relevant creation process. BR, Lizhi
On Wed, 29 Oct 2025 10:59:04 +0800, Lizhi Xu wrote: > > > The root cause of the problem is that multiple different tasks initiate > > > SIOCADDRT & NETROM_NODE commands to add new routes, there is no lock > > > between them to protect the same nr_neigh. > > > > > > Task0 can add the nr_neigh.refcount value of 1 on Task1 to routes[2]. > > > When Task2 executes nr_neigh_put(nr_node->routes[2].neighbour), it will > > > release the neighbour because its refcount value is 1. > > > > > > In this case, the following situation causes a UAF on Task2: > > > > > > Task0 Task1 Task2 > > > ===== ===== ===== > > > nr_add_node() > > > nr_neigh_get_dev() nr_add_node() > > > nr_node_lock() > > > nr_node->routes[2].neighbour->count-- > > > nr_neigh_put(nr_node->routes[2].neighbour); > > > nr_remove_neigh(nr_node->routes[2].neighbour) > > > nr_node_unlock() > > > nr_node_lock() > > > nr_node->routes[2].neighbour = nr_neigh > > > nr_neigh_hold(nr_neigh); nr_add_node() > > > nr_neigh_put() > > > if (nr_node->routes[2].neighbour->count > > > Description of the UAF triggering process: > > > First, Task 0 executes nr_neigh_get_dev() to set neighbor refcount to 3. > > > Then, Task 1 puts the same neighbor from its routes[2] and executes > > > nr_remove_neigh() because the count is 0. After these two operations, > > > the neighbor's refcount becomes 1. Then, Task 0 acquires the nr node > > > lock and writes it to its routes[2].neighbour. > > > Finally, Task 2 executes nr_neigh_put(nr_node->routes[2].neighbour) to > > > release the neighbor. The subsequent execution of the neighbor->count > > > check triggers a UAF. > > > > I looked at the code quite a bit and I think this could possibly avoid > > the above mentioned race, but this whole area looks quite confusing to me. > > > > I think it would be helpful if you could better describe the relevant > > scenario starting from the initial setup (no nodes, no neighs). > OK. Let me fill in the origin of neigh. > > Task3 > ===== > nr_add_node() > [146]if ((nr_neigh = kmalloc(sizeof(*nr_neigh), GFP_ATOMIC)) == NULL) > [253]nr_node->routes[2].neighbour = nr_neigh; > [255]nr_neigh_hold(nr_neigh); > [256]nr_neigh->count++; > > neigh is created on line 146 in nr_add_node(), and added to node on > lines 253-256. It occurs before all Task0, Task1, and Task2. > > Note: > 1. [x], x is line number. > 2. During my debugging process, I didn't pay attention to where the node > was created, and I apologize that I cannot provide the relevant creation > process. Hi everyone, Today is my last day at WindRiver. Starting tomorrow, my email address lizhi.xu@windriver.com will no longer be used; I will use eadavis@qq.com thereafter. BR, Lizhi
© 2016 - 2026 Red Hat, Inc.