linux-kernel - [PATCH] tty: synclink_gt: Fix potential deadlock on &info->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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 28 Jul 2023 12:39:01 +0000
From:   Chengfeng Ye <dg573847474@...il.com>
To:     gregkh@...uxfoundation.org, jirislaby@...nel.org,
        ilpo.jarvinen@...ux.intel.com, bhelgaas@...gle.com,
        russell.h.weight@...el.com
Cc:     linux-kernel@...r.kernel.org, Chengfeng Ye <dg573847474@...il.com>
Subject: [PATCH] tty: synclink_gt: Fix potential deadlock on &info->lock

As &info->lock is acquired by slgt_interrupt() under irq context, other
process context code acquiring the lock 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.

Lock acquisition inside set_params32() does not disable irq, and this
function is called by slgt_compat_ioctl() from process context.

Possible deadlock scenario:
slgt_compat_ioctl()
    -> set_params32()
    -> spin_lock(&info->lock)
        <irq>
        -> slgt_interrupt()
        -> spin_lock(&info->lock); (deadlock here)

This flaw was found by an experimental static analysis tool I am developing
for irq-related deadlock. x86_64 allmodconfig using gcc shows no new
warning.

The patch fixes the potential deadlock by spin_lock_irqsave() like other
lock acquisition sites.

Signed-off-by: Chengfeng Ye <dg573847474@...il.com>
---
 drivers/tty/synclink_gt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/synclink_gt.c b/drivers/tty/synclink_gt.c
index 16e469e581ec..e86bb6228e0f 100644
--- a/drivers/tty/synclink_gt.c
+++ b/drivers/tty/synclink_gt.c
@@ -1088,12 +1088,13 @@ static long get_params32(struct slgt_info *info, struct MGSL_PARAMS32 __user *us
 static long set_params32(struct slgt_info *info, struct MGSL_PARAMS32 __user *new_params)
 {
 	struct MGSL_PARAMS32 tmp_params;
+	unsigned long flags;
 
 	DBGINFO(("%s set_params32\n", info->device_name));
 	if (copy_from_user(&tmp_params, new_params, sizeof(struct MGSL_PARAMS32)))
 		return -EFAULT;
 
-	spin_lock(&info->lock);
+	spin_lock_irqsave(&info->lock, flags);
 	if (tmp_params.mode == MGSL_MODE_BASE_CLOCK) {
 		info->base_clock = tmp_params.clock_speed;
 	} else {
@@ -1111,7 +1112,7 @@ static long set_params32(struct slgt_info *info, struct MGSL_PARAMS32 __user *ne
 		info->params.stop_bits       = tmp_params.stop_bits;
 		info->params.parity          = tmp_params.parity;
 	}
-	spin_unlock(&info->lock);
+	spin_unlock_irqrestore(&info->lock, flags);
 
 	program_hw(info);
 
-- 
2.17.1