Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Alexey Klimov <aklimov@redhat.com>
To:     linux-watchdog@vger.kernel.org
Cc:     wim@linux-watchdog.org, linux@roeck-us.net,
        linux-kernel@vger.kernel.org, klimov.linux@gmail.com
Subject: [PATCH REVIEW 2/2] watchdog: add wd_data->lock mutex locking to watchdog_open()
Date:   Thu, 18 Aug 2022 03:06:24 +0100
Message-Id: <20220818020624.2386475-2-aklimov@redhat.com>
In-Reply-To: <20220818020624.2386475-1-aklimov@redhat.com>
References: <20220818020624.2386475-1-aklimov@redhat.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

watchdog_open() does not have wd_data->lock locks at all unlike
the watchdog_release() for instance. Also watchdog_open() calls
watchdog_start() that should be called with wd_data->lock acquired.
The mutex lock should be acquired in the beginning of the function
after getting struct watchdog_core_data wd_data pointer to deal with
different status fields and be able to call watchdog_start(); and
released on exit and on different error paths.

Signed-off-by: Alexey Klimov <aklimov@redhat.com>
---
 drivers/watchdog/watchdog_dev.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 804236a094f6..d07a864036d9 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -836,7 +836,7 @@ static int watchdog_open(struct inode *inode, struct file *file)
 	struct watchdog_core_data *wd_data;
 	struct watchdog_device *wdd;
 	bool hw_running;
-	int err;
+	int err = -EBUSY;
 
 	/* Get the corresponding watchdog device */
 	if (imajor(inode) == MISC_MAJOR)
@@ -845,9 +845,10 @@ static int watchdog_open(struct inode *inode, struct file *file)
 		wd_data = container_of(inode->i_cdev, struct watchdog_core_data,
 				       cdev);
 
+	mutex_lock(&wd_data->lock);
 	/* the watchdog is single open! */
 	if (test_and_set_bit(_WDOG_DEV_OPEN, &wd_data->status))
-		return -EBUSY;
+		goto out_unlock;
 
 	wdd = wd_data->wdd;
 
@@ -856,10 +857,8 @@ static int watchdog_open(struct inode *inode, struct file *file)
 	 * to be unloaded.
 	 */
 	hw_running = watchdog_hw_running(wdd);
-	if (!hw_running && !try_module_get(wdd->ops->owner)) {
-		err = -EBUSY;
+	if (!hw_running && !try_module_get(wdd->ops->owner))
 		goto out_clear;
-	}
 
 	err = watchdog_start(wdd);
 	if (err < 0)
@@ -878,6 +877,7 @@ static int watchdog_open(struct inode *inode, struct file *file)
 	 * applied.
 	 */
 	wd_data->open_deadline = KTIME_MAX;
+	mutex_unlock(&wd_data->lock);
 
 	/* dev/watchdog is a virtual (and thus non-seekable) filesystem */
 	return stream_open(inode, file);
@@ -886,6 +886,8 @@ static int watchdog_open(struct inode *inode, struct file *file)
 	module_put(wd_data->wdd->ops->owner);
 out_clear:
 	clear_bit(_WDOG_DEV_OPEN, &wd_data->status);
+out_unlock:
+	mutex_unlock(&wd_data->lock);
 	return err;
 }
 
-- 
2.37.2