rose_neigh_put() frees the rose_neigh object when the reference count reaches zero, but does not stop the t0timer and ftimer beforehand. If a timer has been scheduled and fires after the object is freed, the callback will access already-freed memory, leading to a use-after-free. The UAF can be triggered through the following sequence: 1. rose_add_node() creates a rose_neigh with refcount=2 (initial neigh_list reference plus one held by the new node entry) 2. A socket connects via rose_get_neigh(), rose_neigh_hold() raises refcount to 3; rose_transmit_link() arms t0timer on the neigh 3. The routing entry is deleted: rose_del_node() drops the node reference (3->2), calls rose_remove_neigh() which stops both timers, then drops the neigh_list reference (2->1); the socket now holds the only reference with t0timer stopped 4. T1 expires in ROSE_STATE_1: rose_timer_expiry() sends CLEAR_REQUEST via rose_transmit_link(), which sees neigh->restarted=0 and !rose_t0timer_running, and re-arms t0timer on the neigh (refcount still 1) 5. T3 expires in ROSE_STATE_2: rose_timer_expiry() calls rose_neigh_put(), refcount drops to 0, kfree(neigh) 6. t0timer fires, rose_t0timer_expiry() accesses freed neigh -> UAF Calling timer_delete_sync() directly in rose_neigh_put() would fix the UAF but is unsafe: rose_neigh_put() can be called while holding spinlocks (e.g. rose_route_frame() holds rose_neigh_list_lock and rose_route_list_lock), and timer_delete_sync() may block waiting for a running callback, which is illegal in atomic context. Fix this by deferring the final cleanup to a workqueue. When the refcount drops to zero, schedule rose_neigh_free_work() instead of freeing inline. The work function runs in process context where it is safe to call timer_shutdown_sync() before freeing the object. timer_shutdown_sync() is used instead of timer_delete_sync() to permanently prevent the self-rearming t0timer from firing again after the work function starts. Fixes: d860d1faa6b2 ("net: rose: convert 'use' field to refcount_t") Reported-by: syzbot+abd2b69348e2d9b107a1@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=abd2b69348e2d9b107a1 Signed-off-by: Mashiro Chen --- Changes in v3: - Replaced timer_delete_sync() in rose_neigh_put() with a workqueue to avoid calling a blocking function in atomic context - Added rose_neigh_free_work() to handle cleanup in process context - Added struct work_struct free_work to struct rose_neigh - Used timer_shutdown_sync() to prevent the self-rearming t0timer from firing again after cleanup begins Changes in v2: - Stop t0timer and ftimer before freeing the object include/net/rose.h | 10 ++++------ net/rose/rose_route.c | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/include/net/rose.h b/include/net/rose.h index 2b5491bbf39ab..704214e38ef1d 100644 --- a/include/net/rose.h +++ b/include/net/rose.h @@ -9,6 +9,7 @@ #define _ROSE_H #include +#include #include #include #include @@ -105,6 +106,7 @@ struct rose_neigh { struct sk_buff_head queue; struct timer_list t0timer; struct timer_list ftimer; + struct work_struct free_work; }; struct rose_node { @@ -159,12 +161,8 @@ static inline void rose_neigh_hold(struct rose_neigh *rose_neigh) static inline void rose_neigh_put(struct rose_neigh *rose_neigh) { - if (refcount_dec_and_test(&rose_neigh->use)) { - if (rose_neigh->ax25) - ax25_cb_put(rose_neigh->ax25); - kfree(rose_neigh->digipeat); - kfree(rose_neigh); - } + if (refcount_dec_and_test(&rose_neigh->use)) + schedule_work(&rose_neigh->free_work); } /* af_rose.c */ diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c index e31842e6b3c8b..8c8e09793d442 100644 --- a/net/rose/rose_route.c +++ b/net/rose/rose_route.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -44,6 +45,19 @@ static DEFINE_SPINLOCK(rose_route_list_lock); struct rose_neigh *rose_loopback_neigh; +static void rose_neigh_free_work(struct work_struct *work) +{ + struct rose_neigh *rose_neigh = + container_of(work, struct rose_neigh, free_work); + + timer_shutdown_sync(&rose_neigh->t0timer); + timer_shutdown_sync(&rose_neigh->ftimer); + if (rose_neigh->ax25) + ax25_cb_put(rose_neigh->ax25); + kfree(rose_neigh->digipeat); + kfree(rose_neigh); +} + /* * Add a new route to a node, and in the process add the node and the * neighbour if it is new. @@ -103,6 +117,7 @@ static int __must_check rose_add_node(struct rose_route_struct *rose_route, timer_setup(&rose_neigh->ftimer, NULL, 0); timer_setup(&rose_neigh->t0timer, NULL, 0); + INIT_WORK(&rose_neigh->free_work, rose_neigh_free_work); if (rose_route->ndigis != 0) { rose_neigh->digipeat = @@ -388,6 +403,7 @@ void rose_add_loopback_neigh(void) timer_setup(&sn->ftimer, NULL, 0); timer_setup(&sn->t0timer, NULL, 0); + INIT_WORK(&sn->free_work, rose_neigh_free_work); spin_lock_bh(&rose_neigh_list_lock); sn->next = rose_neigh_list; -- 2.53.0