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

[Hyunwoo Kim <imv4bel@gmail.com>]

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 <imv4bel@gmail.com>
---
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