Takashi Iwai da5eef
From 25d5648802f12ae486076ceca5d7ddf1fef792b2 Mon Sep 17 00:00:00 2001
Takashi Iwai da5eef
From: Yang Yingliang <yangyingliang@huawei.com>
Takashi Iwai da5eef
Date: Mon, 5 Dec 2022 16:04:34 +0800
Takashi Iwai da5eef
Subject: [PATCH] w1: fix deadloop in __w1_remove_master_device()
Takashi Iwai da5eef
Git-commit: 25d5648802f12ae486076ceca5d7ddf1fef792b2
Takashi Iwai da5eef
Patch-mainline: v6.2-rc5
Takashi Iwai da5eef
References: git-fixes
Takashi Iwai da5eef
Takashi Iwai da5eef
I got a deadloop report while doing device(ds2482) add/remove test:
Takashi Iwai da5eef
Takashi Iwai da5eef
  [  162.241881] w1_master_driver w1_bus_master1: Waiting for w1_bus_master1 to become free: refcnt=1.
Takashi Iwai da5eef
  [  163.272251] w1_master_driver w1_bus_master1: Waiting for w1_bus_master1 to become free: refcnt=1.
Takashi Iwai da5eef
  [  164.296157] w1_master_driver w1_bus_master1: Waiting for w1_bus_master1 to become free: refcnt=1.
Takashi Iwai da5eef
  ...
Takashi Iwai da5eef
Takashi Iwai da5eef
__w1_remove_master_device() can't return, because the dev->refcnt is not zero.
Takashi Iwai da5eef
Takashi Iwai da5eef
w1_add_master_device()			|
Takashi Iwai da5eef
  w1_alloc_dev()			|
Takashi Iwai da5eef
    atomic_set(&dev->refcnt, 2)		|
Takashi Iwai da5eef
  kthread_run()				|
Takashi Iwai da5eef
					|__w1_remove_master_device()
Takashi Iwai da5eef
					|  kthread_stop()
Takashi Iwai da5eef
  // KTHREAD_SHOULD_STOP is set,	|
Takashi Iwai da5eef
  // threadfn(w1_process) won't be	|
Takashi Iwai da5eef
  // called.				|
Takashi Iwai da5eef
  kthread()				|
Takashi Iwai da5eef
					|  // refcnt will never be 0, it's deadloop.
Takashi Iwai da5eef
					|  while (atomic_read(&dev->refcnt)) {...}
Takashi Iwai da5eef
Takashi Iwai da5eef
After calling w1_add_master_device(), w1_process() is not really
Takashi Iwai da5eef
invoked, before w1_process() starting, if kthread_stop() is called
Takashi Iwai da5eef
in __w1_remove_master_device(), w1_process() will never be called,
Takashi Iwai da5eef
the refcnt can not be decreased, then it causes deadloop in remove
Takashi Iwai da5eef
function because of non-zero refcnt.
Takashi Iwai da5eef
Takashi Iwai da5eef
We need to make sure w1_process() is really started, so move the
Takashi Iwai da5eef
set refcnt into w1_process() to fix this problem.
Takashi Iwai da5eef
Takashi Iwai da5eef
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Takashi Iwai da5eef
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Takashi Iwai da5eef
Link: https://lore.kernel.org/r/20221205080434.3149205-1-yangyingliang@huawei.com
Takashi Iwai da5eef
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Takashi Iwai da5eef
Acked-by: Takashi Iwai <tiwai@suse.de>
Takashi Iwai da5eef
Takashi Iwai da5eef
---
Takashi Iwai da5eef
 drivers/w1/w1.c     | 2 ++
Takashi Iwai da5eef
 drivers/w1/w1_int.c | 5 ++---
Takashi Iwai da5eef
 2 files changed, 4 insertions(+), 3 deletions(-)
Takashi Iwai da5eef
Takashi Iwai da5eef
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
Takashi Iwai da5eef
index f2ae2e563dc5..8b35fae4cd61 100644
Takashi Iwai da5eef
--- a/drivers/w1/w1.c
Takashi Iwai da5eef
+++ b/drivers/w1/w1.c
Takashi Iwai da5eef
@@ -1166,6 +1166,8 @@ int w1_process(void *data)
Takashi Iwai da5eef
 	/* remainder if it woke up early */
Takashi Iwai da5eef
 	unsigned long jremain = 0;
Takashi Iwai da5eef
 
Takashi Iwai da5eef
+	atomic_inc(&dev->refcnt);
Takashi Iwai da5eef
+
Takashi Iwai da5eef
 	for (;;) {
Takashi Iwai da5eef
 
Takashi Iwai da5eef
 		if (!jremain && dev->search_count) {
Takashi Iwai da5eef
diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
Takashi Iwai da5eef
index b3e1792d9c49..3a71c5eb2f83 100644
Takashi Iwai da5eef
--- a/drivers/w1/w1_int.c
Takashi Iwai da5eef
+++ b/drivers/w1/w1_int.c
Takashi Iwai da5eef
@@ -51,10 +51,9 @@ static struct w1_master *w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
Takashi Iwai da5eef
 	dev->search_count	= w1_search_count;
Takashi Iwai da5eef
 	dev->enable_pullup	= w1_enable_pullup;
Takashi Iwai da5eef
 
Takashi Iwai da5eef
-	/* 1 for w1_process to decrement
Takashi Iwai da5eef
-	 * 1 for __w1_remove_master_device to decrement
Takashi Iwai da5eef
+	/* For __w1_remove_master_device to decrement
Takashi Iwai da5eef
 	 */
Takashi Iwai da5eef
-	atomic_set(&dev->refcnt, 2);
Takashi Iwai da5eef
+	atomic_set(&dev->refcnt, 1);
Takashi Iwai da5eef
 
Takashi Iwai da5eef
 	INIT_LIST_HEAD(&dev->slist);
Takashi Iwai da5eef
 	INIT_LIST_HEAD(&dev->async_list);
Takashi Iwai da5eef
-- 
Takashi Iwai da5eef
2.35.3
Takashi Iwai da5eef