Re: [PATCH net v2] rose: fix dangling neighbour pointers in rose_rt_device_down()

[Simon Horman]

On Sun, Jun 29, 2025 at 12:06:31PM +0900, Kohei Enju wrote:
> 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: syzbot+e04e2c007ba2c80476cb@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=e04e2c007ba2c80476cb
> Tested-by: syzbot+e04e2c007ba2c80476cb@xxxxxxxxxxxxxxxxxxxxxxxxx
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Kohei Enju <enjuk@xxxxxxxxxx>
> ---
> Changes:
>   v2:
>     - Change commit message to describe the UAF scenario correctly
>     - Replace for loop with memmove() for array shifting
>   v1: https://lore.kernel.org/all/20250625095005.66148-2-enjuk@xxxxxxxxxx/

Reviewed-by: Simon Horman <horms@xxxxxxxxxx>