[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] watchdog: cpwd: AB-BA deadlock between misc_mtx and cpwd_mutex
--

commit b2d69e49e806eb6cb2c3610179ff17ccacb7cf1b
Author: Muhammad Bilal <meatuni001@gmail.com>

watchdog: cpwd: serialize device access against cpwd_remove()

This commit adds synchronization using cpwd_mutex across device operations 
and teardown. It guards cpwd_remove, cpwd_open, cpwd_ioctl, and cpwd_write 
to prevent race conditions that could lead to use-after-free or NULL pointer 
dereferences.

> diff --git a/drivers/watchdog/cpwd.c b/drivers/watchdog/cpwd.c
> index 13a4d47e68cd8..48f5e92b8cbd0 100644
> --- a/drivers/watchdog/cpwd.c
> +++ b/drivers/watchdog/cpwd.c

[ ... ]

> @@ -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);

[Severity: High]
Could this introduction of guard(mutex)(&cpwd_mutex) here create an AB-BA
deadlock with the misc subsystem's misc_mtx?

When a user opens the device, the misc subsystem acquires misc_mtx before
calling the driver's open function:

drivers/char/misc.c:misc_open() {
    mutex_lock(&misc_mtx);
    ...
    if (file->f_op->open)
        err = file->f_op->open(inode, file);
}

Then, in cpwd_open(), the driver acquires cpwd_mutex:

drivers/watchdog/cpwd.c:cpwd_open() {
    guard(mutex)(&cpwd_mutex);
    ...
}

This establishes the lock order: misc_mtx -> cpwd_mutex.

However, during device removal, this new code acquires cpwd_mutex first,
and then calls misc_deregister(), which attempts to acquire misc_mtx:

drivers/char/misc.c:misc_deregister() {
    mutex_lock(&misc_mtx);
    ...
}

This creates a reverse lock dependency (cpwd_mutex -> misc_mtx), which
could deadlock if a module removal or device unbind occurs concurrently
with a user-space open on the watchdog device.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260919192301.272194-1-meatuni001@gmail.com?part=1

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

[Guenter Roeck]

On Sat, Sep 19, 2026 at 07:33:19PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] watchdog: cpwd: AB-BA deadlock between misc_mtx and cpwd_mutex
> --
> 
> commit b2d69e49e806eb6cb2c3610179ff17ccacb7cf1b
> Author: Muhammad Bilal <meatuni001@gmail.com>
> 
> watchdog: cpwd: serialize device access against cpwd_remove()
> 
> This commit adds synchronization using cpwd_mutex across device operations 
> and teardown. It guards cpwd_remove, cpwd_open, cpwd_ioctl, and cpwd_write 
> to prevent race conditions that could lead to use-after-free or NULL pointer 
> dereferences.
> 

I am not going to apply this patch.

If you want to do anything serious with this driver and you have the hardware
to test the code, please convert it to use the watchdog subsystem. Otherwise
please leave it alone.

Thanks,
Guenter