linux-kernel - Re: [PATCH net v2] 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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250630125158.GG41770@horms.kernel.org>
Date: Mon, 30 Jun 2025 13:51:58 +0100
From: Simon Horman <horms@...nel.org>
To: Kohei Enju <enjuk@...zon.com>
Cc: linux-hams@...r.kernel.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	kohei.enju@...il.com,
	syzbot+e04e2c007ba2c80476cb@...kaller.appspotmail.com
Subject: Re: [PATCH net v2] rose: fix dangling neighbour pointers in
 rose_rt_device_down()

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@...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>
> ---
> 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@amazon.com/

Reviewed-by: Simon Horman <horms@...nel.org>