[PATCH net-next 1/3] rose: fix race between loopback timer and module removal
f6bvp <[email protected]> Mon, 13 Apr 2026 19:42:36 +0200
| Newsgroups | gmane.linux.hams,gmane.linux.network |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> --- 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