Forwarded: [PATCH] netdevsim: fib: replace flush_work() with cancel_work_sync() for fib_event_work

syzbot posted 1 patch 2 weeks, 5 days ago
drivers/net/netdevsim/fib.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Forwarded: [PATCH] netdevsim: fib: replace flush_work() with cancel_work_sync() for fib_event_work
Posted by syzbot 2 weeks, 5 days ago
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] netdevsim: fib: replace flush_work() with cancel_work_sync() for fib_event_work
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master


flush_work() waits for fib_event_work to finish executing.
However, fib_event_work holds fib_lock while running, and
nsim_fib_flush_work() also tries to acquire fib_lock after
flush_work() returns. If fib_event_work takes a long time
or is blocked, flush_work() can wait indefinitely, causing
a hung task.

Replace all occurrences of flush_work(&data->fib_event_work)
with cancel_work_sync(&data->fib_event_work) to safely cancel
any pending work without waiting indefinitely.

This fixes a hung task reported by syzkaller:
  INFO: task kworker/1:8 blocked for more than 159 seconds

The following call chain triggers the hang:
  nsim_fib_flush_work()
    flush_work(&data->fib_event_work)  <- waits forever
      nsim_fib_event_work()
        mutex_lock(&data->fib_lock)    <- held here

Reported-by: syzbot+7c11975a7e4a2735d529@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=7c11975a7e4a2735d529
Signed-off-by: Deepanshu Kartikey <Kartikey406@gmail.com>
---
 drivers/net/netdevsim/fib.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netdevsim/fib.c b/drivers/net/netdevsim/fib.c
index 1a42bdbfaa41..f08517ee583c 100644
--- a/drivers/net/netdevsim/fib.c
+++ b/drivers/net/netdevsim/fib.c
@@ -1094,7 +1094,7 @@ static void nsim_fib_dump_inconsistent(struct notifier_block *nb)
 	struct nsim_fib_rt *fib_rt, *fib_rt_tmp;
 
 	/* Flush the work to make sure there is no race with notifications. */
-	flush_work(&data->fib_event_work);
+	cancel_work_sync(&data->fib_event_work);
 
 	/* The notifier block is still not registered, so we do not need to
 	 * take any locks here.
@@ -1505,7 +1505,7 @@ static void nsim_fib_flush_work(struct work_struct *work)
 	struct nsim_fib_rt *fib_rt, *fib_rt_tmp;
 
 	/* Process pending work. */
-	flush_work(&data->fib_event_work);
+	cancel_work_sync(&data->fib_event_work);
 
 	mutex_lock(&data->fib_lock);
 	list_for_each_entry_safe(fib_rt, fib_rt_tmp, &data->fib_rt_list, list) {
@@ -1626,7 +1626,7 @@ struct nsim_fib_data *nsim_fib_create(struct devlink *devlink,
 	unregister_nexthop_notifier(devlink_net(devlink), &data->nexthop_nb);
 err_rhashtable_fib_destroy:
 	cancel_work_sync(&data->fib_flush_work);
-	flush_work(&data->fib_event_work);
+	cancel_work_sync(&data->fib_event_work);
 	rhashtable_free_and_destroy(&data->fib_rt_ht, nsim_fib_rt_free,
 				    data);
 err_rhashtable_nexthop_destroy:
@@ -1656,7 +1656,7 @@ void nsim_fib_destroy(struct devlink *devlink, struct nsim_fib_data *data)
 	unregister_fib_notifier(devlink_net(devlink), &data->fib_nb);
 	unregister_nexthop_notifier(devlink_net(devlink), &data->nexthop_nb);
 	cancel_work_sync(&data->fib_flush_work);
-	flush_work(&data->fib_event_work);
+	cancel_work_sync(&data->fib_event_work);
 	rhashtable_free_and_destroy(&data->fib_rt_ht, nsim_fib_rt_free,
 				    data);
 	rhashtable_free_and_destroy(&data->nexthop_ht, nsim_nexthop_free,
-- 
2.43.0