[PATCH] rose: Fix use-after-free in rose_timer_expiry

[PATCH] rose: Fix use-after-free in rose_timer_expiry

[Deepanshu Kartikey]

A use-after-free bug can occur when rose_timer_expiry() in state
ROSE_STATE_2 releases the rose_neigh structure via rose_neigh_put(),
while the neighbour's timers (ftimer and t0timer) are still active
or being processed.

The race occurs between:
1. rose_timer_expiry() freeing rose_neigh via rose_neigh_put()
2. rose_t0timer_expiry() attempting to rearm itself via
   rose_start_t0timer(), which calls add_timer() on the freed
   structure

This leads to a KASAN use-after-free report when the timer code
attempts to access the freed memory:

BUG: KASAN: slab-use-after-free in timer_is_static_object+0x80/0x90
Read of size 8 at addr ffff88807e5e8498 by task syz.4.6813/32052

The buggy address is located 152 bytes inside of freed 512-byte
region allocated by rose_add_node().

Fix this by calling timer_shutdown() on both ftimer and t0timer
before releasing the rose_neigh structure. timer_shutdown() ensures
the timers are stopped and prevents them from being rearmed, even
if their callbacks are currently executing.

This fix is based on code analysis as no C reproducer is available
for this issue.

Reported-by: syzbot+62360d745376b40120b5@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 net/rose/rose_timer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/rose/rose_timer.c b/net/rose/rose_timer.c
index bb60a1654d61..6e6483c024fa 100644
--- a/net/rose/rose_timer.c
+++ b/net/rose/rose_timer.c
@@ -180,6 +180,8 @@ static void rose_timer_expiry(struct timer_list *t)
 		break;
 
 	case ROSE_STATE_2:	/* T3 */
+		timer_shutdown(&rose->neighbour->ftimer);
+		timer_shutdown(&rose->neighbour->t0timer);
 		rose_neigh_put(rose->neighbour);
 		rose_disconnect(sk, ETIMEDOUT, -1, -1);
 		break;
-- 
2.43.0

Re: [PATCH] rose: Fix use-after-free in rose_timer_expiry

[F6BVP]

Patch applied to rose_timer.c

Result is interesting even if

rose module cannot still be removed with rmmod command.

For a very long time rmmod would freeze the console and the task could 
not be killed.

With the patch applied rmmod command is not blocked anymore.

However rose module is not removed probably because of a wrong refcount ?

lsmod | grep rose
rose                  184320  -1
ax25                  217088  1 rose

Bernard, f6bvp

[PATCH net-next 1/3] rose: fix race between loopback timer and module removal

[f6bvp]

rose_loopback_clear() used timer_delete() which returns immediately
without waiting for any running callback to complete.  If the timer
fired concurrently with module removal, rose_loopback_timer() would
access rose_loopback_neigh after it was freed, causing a use-after-free.

Three changes fix the race:

1. Add a loopback_stopping atomic flag.  rose_loopback_timer() checks
   this at entry and mid-loop; when set it drains the queue and bails
   out without re-arming the timer.

2. Switch rose_loopback_clear() to timer_delete_sync() so it blocks
   until any in-flight callback has returned.

3. Wrap the timer body with rose_neigh_hold()/rose_neigh_put() so the
   loopback neighbour cannot be freed while the callback is running.

Also fix a pre-existing bug: dev_put(dev) was only called on the
failure path of rose_rx_call_request(); it is now called unconditionally
so the device reference is always released.

Remove a dead check (!neigh->dev && !neigh->loopback) that can never
be true for the loopback neighbour, which always has loopback=1.

Signed-off-by: f6bvp <bernard.f6bvp@gmail.com>
---
 net/rose/rose_loopback.c | 53 +++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
index b538e39b3df5..80d7879ef36a 100644
--- a/net/rose/rose_loopback.c
+++ b/net/rose/rose_loopback.c
@@ -12,13 +12,15 @@
 #include <net/rose.h>
 #include <linux/init.h>
 
-static struct sk_buff_head loopback_queue;
 #define ROSE_LOOPBACK_LIMIT 1000
-static struct timer_list loopback_timer;
 
+static struct timer_list loopback_timer;
+static struct sk_buff_head loopback_queue;
 static void rose_set_loopback_timer(void);
 static void rose_loopback_timer(struct timer_list *unused);
 
+static atomic_t loopback_stopping = ATOMIC_INIT(0);
+
 void rose_loopback_init(void)
 {
 	skb_queue_head_init(&loopback_queue);
@@ -66,10 +68,25 @@ static void rose_loopback_timer(struct timer_list *unused)
 	unsigned int lci_i, lci_o;
 	int count;
 
+	if (atomic_read(&loopback_stopping))
+		return;
+
+	if (rose_loopback_neigh)
+		rose_neigh_hold(rose_loopback_neigh);
+	else
+		return;
+
 	for (count = 0; count < ROSE_LOOPBACK_LIMIT; count++) {
 		skb = skb_dequeue(&loopback_queue);
 		if (!skb)
-			return;
+			goto out;
+
+		if (atomic_read(&loopback_stopping)) {
+			kfree_skb(skb);
+			skb_queue_purge(&loopback_queue);
+			goto out;
+		}
+
 		if (skb->len < ROSE_MIN_LEN) {
 			kfree_skb(skb);
 			continue;
@@ -96,27 +113,24 @@ static void rose_loopback_timer(struct timer_list *unused)
 		}
 
 		if (frametype == ROSE_CALL_REQUEST) {
-			if (!rose_loopback_neigh->dev &&
-			    !rose_loopback_neigh->loopback) {
-				kfree_skb(skb);
-				continue;
-			}
-
 			dev = rose_dev_get(dest);
 			if (!dev) {
 				kfree_skb(skb);
 				continue;
 			}
 
-			if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0) {
-				dev_put(dev);
+			if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0)
 				kfree_skb(skb);
-			}
+			dev_put(dev);
 		} else {
 			kfree_skb(skb);
 		}
 	}
-	if (!skb_queue_empty(&loopback_queue))
+
+out:
+	rose_neigh_put(rose_loopback_neigh);
+
+	if (!atomic_read(&loopback_stopping) && !skb_queue_empty(&loopback_queue))
 		mod_timer(&loopback_timer, jiffies + 1);
 }
 
@@ -124,10 +138,15 @@ void __exit rose_loopback_clear(void)
 {
 	struct sk_buff *skb;
 
-	timer_delete(&loopback_timer);
+	atomic_set(&loopback_stopping, 1);
+	/* Pairs with atomic_read() in rose_loopback_timer(): ensure the
+	 * stopping flag is visible before we cancel, so a concurrent
+	 * callback aborts its loop early rather than re-arming the timer.
+	 */
+	smp_mb();
 
-	while ((skb = skb_dequeue(&loopback_queue)) != NULL) {
-		skb->sk = NULL;
+	timer_delete_sync(&loopback_timer);
+
+	while ((skb = skb_dequeue(&loopback_queue)) != NULL)
 		kfree_skb(skb);
-	}
 }
-- 
2.51.0

[PATCH net-next 2/3] rose: clear neighbour pointer after rose_neigh_put() in state machines

[f6bvp]

After releasing a neighbour reference with rose_neigh_put() in the
ROSE state machines, the pointer in rose_sock was left dangling.
A subsequent code path could dereference the freed neighbour, causing
a use-after-free.

Set rose->neighbour to NULL immediately after each rose_neigh_put()
call in rose_state1_machine() through rose_state5_machine().

Signed-off-by: f6bvp <bernard.f6bvp@gmail.com>
---
 net/rose/rose_in.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/rose/rose_in.c b/net/rose/rose_in.c
index 0276b393f0e5..622527f1354f 100644
--- a/net/rose/rose_in.c
+++ b/net/rose/rose_in.c
@@ -57,6 +57,7 @@ static int rose_state1_machine(struct sock *sk, struct sk_buff *skb, int framety
 		rose_write_internal(sk, ROSE_CLEAR_CONFIRMATION);
 		rose_disconnect(sk, ECONNREFUSED, skb->data[3], skb->data[4]);
 		rose_neigh_put(rose->neighbour);
+		rose->neighbour = NULL;
 		break;
 
 	default:
@@ -80,11 +81,13 @@ static int rose_state2_machine(struct sock *sk, struct sk_buff *skb, int framety
 		rose_write_internal(sk, ROSE_CLEAR_CONFIRMATION);
 		rose_disconnect(sk, 0, skb->data[3], skb->data[4]);
 		rose_neigh_put(rose->neighbour);
+		rose->neighbour = NULL;
 		break;
 
 	case ROSE_CLEAR_CONFIRMATION:
 		rose_disconnect(sk, 0, -1, -1);
 		rose_neigh_put(rose->neighbour);
+		rose->neighbour = NULL;
 		break;
 
 	default:
@@ -122,6 +125,7 @@ static int rose_state3_machine(struct sock *sk, struct sk_buff *skb, int framety
 		rose_write_internal(sk, ROSE_CLEAR_CONFIRMATION);
 		rose_disconnect(sk, 0, skb->data[3], skb->data[4]);
 		rose_neigh_put(rose->neighbour);
+		rose->neighbour = NULL;
 		break;
 
 	case ROSE_RR:
@@ -235,6 +239,7 @@ static int rose_state4_machine(struct sock *sk, struct sk_buff *skb, int framety
 		rose_write_internal(sk, ROSE_CLEAR_CONFIRMATION);
 		rose_disconnect(sk, 0, skb->data[3], skb->data[4]);
 		rose_neigh_put(rose->neighbour);
+		rose->neighbour = NULL;
 		break;
 
 	default:
@@ -255,6 +260,7 @@ static int rose_state5_machine(struct sock *sk, struct sk_buff *skb, int framety
 		rose_write_internal(sk, ROSE_CLEAR_CONFIRMATION);
 		rose_disconnect(sk, 0, skb->data[3], skb->data[4]);
 		rose_neigh_put(rose_sk(sk)->neighbour);
+		rose_sk(sk)->neighbour = NULL;
 	}
 
 	return 0;
-- 
2.51.0

Re: [PATCH net-next 2/3] rose: clear neighbour pointer after rose_neigh_put() in state machines

[Jakub Kicinski]

On Mon, 13 Apr 2026 19:42:37 +0200 f6bvp wrote:
> Signed-off-by: f6bvp <bernard.f6bvp@gmail.com>

Human name is required when authoring patches.
Please do not post patches in reply to existing threads. See:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
-- 
pw-bot: cr

[PATCH net-next 3/3] rose: guard rose_neigh_put() against NULL in timer expiry

[f6bvp]

In rose_timer_expiry(), ROSE_STATE_2 calls rose_neigh_put() on
rose->neighbour without checking whether it is NULL first.  The pointer
can be NULL if the connection was already being torn down by a
concurrent code path (e.g. rose_kill_by_neigh()), leading to a
NULL-pointer dereference.

Add a NULL check before the put and clear the pointer afterwards.

Signed-off-by: f6bvp <bernard.f6bvp@gmail.com>
---
 net/rose/rose_timer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/rose/rose_timer.c b/net/rose/rose_timer.c
index bb60a1654d61..d997d24ab081 100644
--- a/net/rose/rose_timer.c
+++ b/net/rose/rose_timer.c
@@ -180,7 +180,10 @@ static void rose_timer_expiry(struct timer_list *t)
 		break;
 
 	case ROSE_STATE_2:	/* T3 */
-		rose_neigh_put(rose->neighbour);
+		if (rose->neighbour) {
+			rose_neigh_put(rose->neighbour);
+			rose->neighbour = NULL;
+		}
 		rose_disconnect(sk, ETIMEDOUT, -1, -1);
 		break;
 
-- 
2.51.0

Re: [PATCH net-next 1/3] rose: fix race between loopback timer and module removal

[Andrew Lunn]

On Mon, Apr 13, 2026 at 07:42:36PM +0200, f6bvp wrote:
> rose_loopback_clear() used timer_delete() which returns immediately
> without waiting for any running callback to complete.  If the timer
> fired concurrently with module removal, rose_loopback_timer() would
> access rose_loopback_neigh after it was freed, causing a use-after-free.
> 
> Three changes fix the race:
> 
> 1. Add a loopback_stopping atomic flag.  rose_loopback_timer() checks
>    this at entry and mid-loop; when set it drains the queue and bails
>    out without re-arming the timer.
> 
> 2. Switch rose_loopback_clear() to timer_delete_sync() so it blocks
>    until any in-flight callback has returned.
> 
> 3. Wrap the timer body with rose_neigh_hold()/rose_neigh_put() so the
>    loopback neighbour cannot be freed while the callback is running.
> 
> Also fix a pre-existing bug: dev_put(dev) was only called on the
> failure path of rose_rx_call_request(); it is now called unconditionally
> so the device reference is always released.

Hi Barnard

Thanks for the patches.

A few process points.

We prefer lots of small patches, with good commit messages, which are
obviously correct. When i see a list like this, it makes me think the
patch can be split up into smaller patches.

When you have a patch series, please include a patch 0/X which
explains the big picture.

net-next is current closed for the merge window. You can post patches
as RFC, but don't post anything expecting it to be merged.

There is more information here:

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

	Andrew

Re: [PATCH net-next 1/3] rose: fix race between loopback timer and module removal

[Jakub Kicinski]

On Mon, 13 Apr 2026 23:21:16 +0200 Andrew Lunn wrote:
> net-next is current closed for the merge window. You can post patches
> as RFC, but don't post anything expecting it to be merged.

FWIW I assumed this were fixes, in which case the closing of the trees
would not apply, the patches should be marked as "PATCH net" and have 
a Fixes tag.