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: 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 --- 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.0