netdev - 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: <20251029025904.63619-1-lizhi.xu@windriver.com>
Date: Wed, 29 Oct 2025 10:59:04 +0800
From: Lizhi Xu <lizhi.xu@...driver.com>
To: <pabeni@...hat.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>, <lizhi.xu@...driver.com>,
        <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 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