[PATCH] watchdog: cpwd: serialize device access against cpwd_remove()

[PATCH] watchdog: cpwd: serialize device access against cpwd_remove()

[Muhammad Bilal]

cpwd_open(), cpwd_ioctl() and cpwd_write() all read the global
cpwd_device pointer directly, while cpwd_remove() unmaps p->regs and
then sets cpwd_device = NULL with no locking shared with those paths.
misc_deregister() does not revoke file descriptors that are already
open, so a process holding one across an unbind/remove can still
reach cpwd_ioctl()/cpwd_write() afterwards: it will either dereference
p->regs after of_iounmap() has torn it down, or run into a NULL p once
cpwd_device has been cleared.

Take the existing cpwd_mutex around the whole of cpwd_remove()'s
teardown, and have cpwd_open()/cpwd_ioctl()/cpwd_write() re-read
cpwd_device under the same mutex and bail out with -ENODEV when it is
NULL. This makes any in-flight call finish (or fail cleanly) strictly
before or after teardown, instead of racing it.

Fixes: 8ab0dc333eac ("cpwatchdog: Move to drivers/watchdog/cpwd.c")
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/watchdog/cpwd.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/cpwd.c b/drivers/watchdog/cpwd.c
index 13a4d47e68cd..48f5e92b8cbd 100644
--- a/drivers/watchdog/cpwd.c
+++ b/drivers/watchdog/cpwd.c
@@ -368,9 +368,14 @@ static irqreturn_t cpwd_interrupt(int irq, void *dev_id)
 
 static int cpwd_open(struct inode *inode, struct file *f)
 {
-	struct cpwd *p = cpwd_device;
+	struct cpwd *p;
+
+	guard(mutex)(&cpwd_mutex);
+
+	p = cpwd_device;
+	if (!p)
+		return -ENODEV;
 
-	mutex_lock(&cpwd_mutex);
 	switch (iminor(inode)) {
 	case WD0_MINOR:
 	case WD1_MINOR:
@@ -378,7 +383,6 @@ static int cpwd_open(struct inode *inode, struct file *f)
 		break;
 
 	default:
-		mutex_unlock(&cpwd_mutex);
 		return -ENODEV;
 	}
 
@@ -387,14 +391,11 @@ static int cpwd_open(struct inode *inode, struct file *f)
 		if (request_irq(p->irq, &cpwd_interrupt,
 				IRQF_SHARED, DRIVER_NAME, p)) {
 			pr_err("Cannot register IRQ %d\n", p->irq);
-			mutex_unlock(&cpwd_mutex);
 			return -EBUSY;
 		}
 		p->initialized = true;
 	}
 
-	mutex_unlock(&cpwd_mutex);
-
 	return stream_open(inode, f);
 }
 
@@ -413,9 +414,15 @@ static long cpwd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	void __user *argp = (void __user *)arg;
 	struct inode *inode = file_inode(file);
 	int index = iminor(inode) - WD0_MINOR;
-	struct cpwd *p = cpwd_device;
+	struct cpwd *p;
 	int setopt = 0;
 
+	guard(mutex)(&cpwd_mutex);
+
+	p = cpwd_device;
+	if (!p)
+		return -ENODEV;
+
 	switch (cmd) {
 	/* Generic Linux IOCTLs */
 	case WDIOC_GETSUPPORT:
@@ -482,8 +489,14 @@ static ssize_t cpwd_write(struct file *file, const char __user *buf,
 			  size_t count, loff_t *ppos)
 {
 	struct inode *inode = file_inode(file);
-	struct cpwd *p = cpwd_device;
 	int index = iminor(inode);
+	struct cpwd *p;
+
+	guard(mutex)(&cpwd_mutex);
+
+	p = cpwd_device;
+	if (!p)
+		return -ENODEV;
 
 	if (count) {
 		cpwd_pingtimer(p, index);
@@ -618,6 +631,8 @@ static void cpwd_remove(struct platform_device *op)
 	struct cpwd *p = platform_get_drvdata(op);
 	int i;
 
+	guard(mutex)(&cpwd_mutex);
+
 	for (i = 0; i < WD_NUMDEVS; i++) {
 		misc_deregister(&p->devs[i].misc);
 
-- 
2.55.0