Re: [PATCH V2] net: rose: Prevent the use of freed digipeat
From: Eric Dumazet <edumazet@google.com>
Date: 2025-10-24 09:19:58
Also in:
linux-hams, lkml
On Fri, Oct 24, 2025 at 2:06 AM Lizhi Xu [off-list ref] wrote:
quoted hunk ↗ jump to hunk
There is no synchronization between the two timers, rose_t0timer_expiry and rose_timer_expiry. rose_timer_expiry() puts the neighbor when the rose state is ROSE_STATE_2. However, rose_t0timer_expiry() does initiate a restart request on the neighbor. When rose_t0timer_expiry() accesses the released neighbor member digipeat, a UAF is triggered. To avoid this UAF, defer the put operation to rose_t0timer_expiry() and stop restarting t0timer after putting the neighbor. When putting the neighbor, set the neighbor to NULL. Setting neighbor to NULL prevents rose_t0timer_expiry() from restarting t0timer. syzbot reported a slab-use-after-free Read in ax25_find_cb. BUG: KASAN: slab-use-after-free in ax25_find_cb+0x3b8/0x3f0 net/ax25/af_ax25.c:237 Read of size 1 at addr ffff888059c704c0 by task syz.6.2733/17200 Call Trace: ax25_find_cb+0x3b8/0x3f0 net/ax25/af_ax25.c:237 ax25_send_frame+0x157/0xb60 net/ax25/ax25_out.c:55 rose_send_frame+0xcc/0x2c0 net/rose/rose_link.c:106 rose_transmit_restart_request+0x1b8/0x240 net/rose/rose_link.c:198 rose_t0timer_expiry+0x1d/0x150 net/rose/rose_link.c:83 Freed by task 17183: kfree+0x2b8/0x6d0 mm/slub.c:6826 rose_neigh_put include/net/rose.h:165 [inline] rose_timer_expiry+0x537/0x630 net/rose/rose_timer.c:183 Fixes: d860d1faa6b2 ("net: rose: convert 'use' field to refcount_t") Reported-by: syzbot+caa052a0958a9146870d@syzkaller.appspotmail.com Signed-off-by: Lizhi Xu <redacted> --- V1 -> V2: Putting the neighbor stops t0timer from automatically starting include/net/rose.h | 1 + net/rose/rose_link.c | 5 +++++ 2 files changed, 6 insertions(+)diff --git a/include/net/rose.h b/include/net/rose.h index 2b5491bbf39a..ecf37c8e24bb 100644 --- a/include/net/rose.h +++ b/include/net/rose.h@@ -164,6 +164,7 @@ static inline void rose_neigh_put(struct rose_neigh *rose_neigh) ax25_cb_put(rose_neigh->ax25); kfree(rose_neigh->digipeat); kfree(rose_neigh); + rose_neigh = NULL;
What is the purpose of this added line ? @rose_neigh is a local variable. Setting it to NULL while this function no longer uses it is going to be optimized out by the compiler. Even if not optimized, this has no effect.
quoted hunk ↗ jump to hunk
} }diff --git a/net/rose/rose_link.c b/net/rose/rose_link.c index 7746229fdc8c..524e7935bd02 100644 --- a/net/rose/rose_link.c +++ b/net/rose/rose_link.c@@ -43,6 +43,9 @@ void rose_start_ftimer(struct rose_neigh *neigh) static void rose_start_t0timer(struct rose_neigh *neigh) { + if (!neigh) + return;
This will never fire. callers would have crashed already it neigh was NULL.
quoted hunk ↗ jump to hunk
+ timer_delete(&neigh->t0timer); neigh->t0timer.function = rose_t0timer_expiry;@@ -80,10 +83,12 @@ static void rose_t0timer_expiry(struct timer_list *t) { struct rose_neigh *neigh = timer_container_of(neigh, t, t0timer);
Can you explain (in a comment) why this is needed ? What is the precise scenario you want to avoid ?
+ rose_neigh_hold(neigh);
rose_transmit_restart_request(neigh);
neigh->dce_mode = 0;
+ rose_neigh_put(neigh);I am pretty sure this patch fixes nothing at all.
rose_start_t0timer(neigh); } -- 2.43.0