All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hyunwoo Kim <imv4bel@gmail.com>
To: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org
Cc: sd@queasysnail.net, nate.karstens@garmin.com, linux@treblig.org,
	Julia.Lawall@inria.fr, netdev@vger.kernel.org, imv4bel@gmail.com
Subject: [PATCH] strparser: Use worker disable API instead of cancellation in strp_done()
Date: Mon, 16 Feb 2026 18:48:08 +0900	[thread overview]
Message-ID: <aZLn2Faeg1FB7XOf@v4bel> (raw)

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.

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

Fixes: 829385f08ae9 ("strparser: Use delayed work instead of timer for msg timeout")
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 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

---
Dear,

The following is a simplified scenario illustrating how each race can occur. Since espintcp_close() does not hold lock_sock(), the race is possible.
Although cancel_work_sync(&strp->work) does not appear to be easy to trigger in practice here, it still seems better to fix it as well.
```
                 cpu0                                cpu1

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

espintcp_close()
                                          sk->sk_data_ready()
                                            espintcp_data_ready()
                                              if (unlikely(strp->stopped)) return;
  strp_stop()
    strp->stopped = 1;
  strp_done()
    cancel_work_sync(&strp->work);
                                              if (strp_read_sock(strp) == -ENOMEM)
                                              queue_work()
```

Best regards,
Hyunwoo Kim

             reply	other threads:[~2026-02-16  9:48 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-16  9:48 Hyunwoo Kim [this message]
2026-02-17 18:21 ` [PATCH] strparser: Use worker disable API instead of cancellation in strp_done() Sabrina Dubroca
2026-02-17 19:45   ` Hyunwoo Kim
2026-02-18 18:11     ` Sabrina Dubroca

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aZLn2Faeg1FB7XOf@v4bel \
    --to=imv4bel@gmail.com \
    --cc=Julia.Lawall@inria.fr \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux@treblig.org \
    --cc=nate.karstens@garmin.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sd@queasysnail.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.