Skip to content

Commit 34a500c

Browse files
hyperenjukuba-moo
authored andcommitted
rose: fix dangling neighbour pointers in rose_rt_device_down()
There are two bugs in rose_rt_device_down() that can cause 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, A, B) with count=3 and A is being removed, the second A is not checked. i=0: (A, A, B) -> (A, B) with count=2 ^ checked i=1: (A, B) -> (A, B) with count=2 ^ checked (B, not A!) i=2: (doesn't occur because i < count is false) This leaves the second A in the array with count=2, but the rose_neigh structure has been freed. Code that accesses these entries 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 subsequent iterations. Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=e04e2c007ba2c80476cb Tested-by: [email protected] Fixes: 1da177e ("Linux-2.6.12-rc2") Signed-off-by: Kohei Enju <[email protected]> Reviewed-by: Simon Horman <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent aaf2b24 commit 34a500c

File tree

1 file changed

+4
-11
lines changed

1 file changed

+4
-11
lines changed

net/rose/rose_route.c

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -497,22 +497,15 @@ void rose_rt_device_down(struct net_device *dev)
497497
t = rose_node;
498498
rose_node = rose_node->next;
499499

500-
for (i = 0; i < t->count; i++) {
500+
for (i = t->count - 1; i >= 0; i--) {
501501
if (t->neighbour[i] != s)
502502
continue;
503503

504504
t->count--;
505505

506-
switch (i) {
507-
case 0:
508-
t->neighbour[0] = t->neighbour[1];
509-
fallthrough;
510-
case 1:
511-
t->neighbour[1] = t->neighbour[2];
512-
break;
513-
case 2:
514-
break;
515-
}
506+
memmove(&t->neighbour[i], &t->neighbour[i + 1],
507+
sizeof(t->neighbour[0]) *
508+
(t->count - i));
516509
}
517510

518511
if (t->count <= 0)

0 commit comments

Comments
 (0)