linux-kernel - Re: [PATCH V4] netrom: Preventing the use of abnormal neighbor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251113063325.1138331-1-lizhi.xu@windriver.com>
Date: Thu, 13 Nov 2025 14:33:25 +0800
From: Lizhi Xu <lizhi.xu@...driver.com>
To: <lizhi.xu@...driver.com>
CC: <dan.carpenter@...aro.org>, <davem@...emloft.net>, <edumazet@...gle.com>,
        <horms@...nel.org>, <kuba@...nel.org>, <linux-hams@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
        <syzbot+2860e75836a08b172755@...kaller.appspotmail.com>,
        <syzkaller-bugs@...glegroups.com>
Subject: Re: [PATCH V4] netrom: Preventing the use of abnormal neighbor

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@...driver.com will no longer be used;
I will use eadavis@...com thereafter.

BR,
Lizhi