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 --- Changes in v2: - Update Fixes tag - Incorporate Simon’s review: https://lore.kernel.org/all/aZSLHaFj7k8DPmLG@horms.kernel.org/ - 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/ --- 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