linux-kernel - [PATCH] watchdog: s3c2410: Fix potential deadlock on &wdt->lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 28 Jun 2023 16:47:59 +0000
From:   Chengfeng Ye <dg573847474@...il.com>
To:     krzysztof.kozlowski@...aro.org, alim.akhtar@...sung.com,
        wim@...ux-watchdog.org, linux@...ck-us.net
Cc:     linux-arm-kernel@...ts.infradead.org,
        linux-samsung-soc@...r.kernel.org, linux-watchdog@...r.kernel.org,
        linux-kernel@...r.kernel.org, Chengfeng Ye <dg573847474@...il.com>
Subject: [PATCH] watchdog: s3c2410: Fix potential deadlock on &wdt->lock

As &wdt->lock is acquired by hard irq s3c2410wdt_irq(),
other acquisition of the same lock under process context should
disable irq, otherwise deadlock could happen if the
irq preempt the execution while the lock is held in process context
on the same CPU.

  [Interrupt]: s3c2410wdt_irq
    -->/root/linux/drivers/watchdog/s3c2410_wdt.c:547
    -->/root/linux/drivers/watchdog/s3c2410_wdt.c:383
    -->spin_lock(&wdt->lock);

  [Process Context]: s3c2410wdt_suspend
    -->/root/linux/drivers/watchdog/s3c2410_wdt.c:764
    -->/root/linux/drivers/watchdog/s3c2410_wdt.c:403
    -->spin_lock(&wdt->lock);

  [Process Context]: s3c2410wdt_probe
    -->/root/linux/drivers/watchdog/s3c2410_wdt.c:710
    -->/root/linux/drivers/watchdog/s3c2410_wdt.c:415
    -->spin_lock(&wdt->lock);

This flaw was found by an experimental static analysis tool I am
developing for irq-related deadlock, which reported the above
warning when analyzing the linux kernel 6.4-rc7 release.

The tentative patch fix the potential deadlock by spin_lock_irqsave().

Signed-off-by: Chengfeng Ye <dg573847474@...il.com>
---
 drivers/watchdog/s3c2410_wdt.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 95416a9bdd4b..2dfc0d6a3004 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -399,10 +399,11 @@ static void __s3c2410wdt_stop(struct s3c2410_wdt *wdt)
 static int s3c2410wdt_stop(struct watchdog_device *wdd)
 {
 	struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd);
+	unsigned long flags;
 
-	spin_lock(&wdt->lock);
+	spin_lock_irqsave(&wdt->lock, flags);
 	__s3c2410wdt_stop(wdt);
-	spin_unlock(&wdt->lock);
+	spin_unlock_irqrestore(&wdt->lock, flags);
 
 	return 0;
 }
@@ -411,8 +412,9 @@ static int s3c2410wdt_start(struct watchdog_device *wdd)
 {
 	unsigned long wtcon;
 	struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd);
+	unsigned long flags;
 
-	spin_lock(&wdt->lock);
+	spin_lock_irqsave(&wdt->lock, flags);
 
 	__s3c2410wdt_stop(wdt);
 
@@ -433,7 +435,7 @@ static int s3c2410wdt_start(struct watchdog_device *wdd)
 	writel(wdt->count, wdt->reg_base + S3C2410_WTDAT);
 	writel(wdt->count, wdt->reg_base + S3C2410_WTCNT);
 	writel(wtcon, wdt->reg_base + S3C2410_WTCON);
-	spin_unlock(&wdt->lock);
+	spin_unlock_irqrestore(&wdt->lock, flags);
 
 	return 0;
 }
-- 
2.17.1