Thread (13 messages) flat view 13 messages, 3 authors, 2026-04-29
STALE101d

[PATCH net v2] strparser: Fix race condition in strp_done()

From: Hyunwoo Kim <hidden>
Date: 2026-02-20 09:30:00
Subsystem: networking [general], the rest · Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

This issue was discovered during a code audit.

When strp_stop() and strp_done() are called without holding lock_sock(), 
they can race with worker-scheduling paths such as the Delayed ACK handler 
and ksoftirqd.
Specifically, after cancel_delayed_work_sync() and cancel_work_sync() are 
invoked from strp_done(), the workers may still be scheduled.
As a result, the workers may dereference freed objects.

The following is a simple race scenario:

          cpu0                      cpu1

espintcp_close()
                           espintcp_data_ready()
                             strp_data_ready()
                               if (unlikely(strp->stopped)) return;
  strp_stop()
    strp->stopped = 1;
  strp_done()
    cancel_delayed_work_sync(&strp->msg_timer_work);
                               strp_read_sock()
                                 tcp_read_sock()
                                   __tcp_read_sock()
                                     strp_recv()
                                       __strp_recv()
                                         strp_start_timer()
                                           mod_delayed_work(&strp->msg_timer_work);

To prevent these races, the cancellation APIs are replaced with 
worker-disabling APIs.

Fixes: bbb03029a899 ("strparser: Generalize strparser")
Signed-off-by: Hyunwoo Kim <redacted>
---
Changes in v2:
- Update Fixes tag
- Incorporate Simon’s review: https://lore.kernel.org/all/aZSLHaFj7k8DPmLG@horms.kernel.org/ (local)
  - Shorten the patch subject
  - Target the net tree
  - Add the bug discovery background and the race scenario to the commit message
- v1: https://lore.kernel.org/all/aZLn2Faeg1FB7XOf@v4bel/ (local)
---
 net/strparser/strparser.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
index fe0e76fdd1f1..15cd9cadbd1a 100644
--- a/net/strparser/strparser.c
+++ b/net/strparser/strparser.c
@@ -503,8 +503,8 @@ void strp_done(struct strparser *strp)
 {
 	WARN_ON(!strp->stopped);
 
-	cancel_delayed_work_sync(&strp->msg_timer_work);
-	cancel_work_sync(&strp->work);
+	disable_delayed_work_sync(&strp->msg_timer_work);
+	disable_work_sync(&strp->work);
 
 	if (strp->skb_head) {
 		kfree_skb(strp->skb_head);
-- 
2.43.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help