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 --- 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