netdev - [PATCH net v1] rose: fix dangling neighbour pointers in rose_rt_device

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-next>] [day] [month] [year] [list]

Message-ID: <20250625095005.66148-2-enjuk@amazon.com>
Date: Wed, 25 Jun 2025 18:49:44 +0900
From: Kohei Enju <enjuk@...zon.com>
To: <netdev@...r.kernel.org>, <linux-hams@...r.kernel.org>
CC: "David S . Miller" <davem@...emloft.net>, Eric Dumazet
	<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Kuniyuki Iwashima
	<kuniyu@...gle.com>, Ingo Molnar <mingo@...nel.org>, Thomas Gleixner
	<tglx@...utronix.de>, Kohei Enju <kohei.enju@...il.com>, Kohei Enju
	<enjuk@...zon.com>, <syzbot+e04e2c007ba2c80476cb@...kaller.appspotmail.com>
Subject: [PATCH net v1] rose: fix dangling neighbour pointers in rose_rt_device_down()

There are two bugs in rose_rt_device_down() that can lead to
use-after-free:

1. The loop bound `t->count` is modified within the loop, which can
   cause the loop to terminate early and miss some entries.

2. When removing an entry from the neighbour array, the subsequent entries
   are moved up to fill the gap, but the loop index `i` is still
   incremented, causing the next entry to be skipped.

For example, if a node has three neighbours (A, B, A) and A is being
removed:
- 1st iteration (i=0): A is removed, array becomes (B, A, A), count=2
- 2nd iteration (i=1): We now check A instead of B, skipping B entirely
- 3rd iteration (i=2): Loop terminates early due to count=2

This leaves the second A in the array with count=2, but the rose_neigh
structure has been freed. Accessing code assumes that the first `count`
entries are valid pointers, causing a use-after-free when it accesses
the dangling pointer.

Fix both issues by iterating over the array in reverse order with a fixed
loop bound. This ensures that all entries are examined and that the removal
of an entry doesn't affect the iteration of subsequent entries.

Reported-by: syzbot+e04e2c007ba2c80476cb@...kaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e04e2c007ba2c80476cb
Tested-by: syzbot+e04e2c007ba2c80476cb@...kaller.appspotmail.com
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kohei Enju <enjuk@...zon.com>
---
 net/rose/rose_route.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 2dd6bd3a3011..a488fd8c4710 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -479,7 +479,7 @@ void rose_rt_device_down(struct net_device *dev)
 {
 	struct rose_neigh *s, *rose_neigh;
 	struct rose_node  *t, *rose_node;
-	int i;
+	int i, j;
 
 	spin_lock_bh(&rose_node_list_lock);
 	spin_lock_bh(&rose_neigh_list_lock);
@@ -497,22 +497,14 @@ void rose_rt_device_down(struct net_device *dev)
 			t         = rose_node;
 			rose_node = rose_node->next;
 
-			for (i = 0; i < t->count; i++) {
+			for (i = t->count - 1; i >= 0; i--) {
 				if (t->neighbour[i] != s)
 					continue;
 
 				t->count--;
 
-				switch (i) {
-				case 0:
-					t->neighbour[0] = t->neighbour[1];
-					fallthrough;
-				case 1:
-					t->neighbour[1] = t->neighbour[2];
-					break;
-				case 2:
-					break;
-				}
+				for (j = i; j < t->count; j++)
+					t->neighbour[j] = t->neighbour[j + 1];
 			}
 
 			if (t->count <= 0)
-- 
2.48.1