Skip to contentSkip to conversation

gpiolib: cdev: Fix &lr->wait.lock deadlock issue

PATCH New Chengfeng Ye 1 participant 2 messages Jun 26, 2023

Subsystems (2)
GPIO GPIO uapi
lore
Share & tools
lore

Chengfeng Ye

author
#0 lore raw

PATCH v2 1/2 gpiolib: cdev: Fix &lr->wait.lock deadlock issue

linereq_put_event() is called from both interrupt context (e.g., edge_irq_thread()) and process context (process_hw_ts_thread()). Therefore, interrupt should be disabled before acquiring lock &lr->wait.lock inside linereq_put_event() to avoid deadlock when the lock is held in process context and edge_irq_thread() comes.

Similarly, linereq_read_unlocked() running in process context also acquies the same lock. It also need to disable interrupt otherwise deadlock could happen if the irq edge_irq_thread() comes to execution while the lock is held.

Fix the two potential deadlock issues by spin_lock_bh() and spin_lock_irq() separately.

Fixes 73e0341992b6 ("gpiolib: cdev: support edge detection for uAPI v2")
Signed-off-by Chengfeng Ye <[email protected]>
---
 drivers/gpio/gpiolib-cdev.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/gpio/gpiolib-cdev.c +8 -8
index 0a33971c964c..f768d46bdea7 100644
@@ -615,13 +615,13 @@ static void linereq_put_event(struct linereq *lr,
 {
 	bool overflow = false;
 
-	spin_lock(&lr->wait.lock);
+	spin_lock_bh(&lr->wait.lock);
 	if (kfifo_is_full(&lr->events)) {
 		overflow = true;
 		kfifo_skip(&lr->events);
 	}
 	kfifo_in(&lr->events, le, 1);
-	spin_unlock(&lr->wait.lock);
+	spin_unlock_bh(&lr->wait.lock);
 	if (!overflow)
 		wake_up_poll(&lr->wait, EPOLLIN);
 	else

@@ -1514,28 +1514,28 @@ static ssize_t linereq_read_unlocked(struct file *file, char __user *buf,
 		return -EINVAL;
 
 	do {
-		spin_lock(&lr->wait.lock);
+		spin_lock_irq(&lr->wait.lock);
 		if (kfifo_is_empty(&lr->events)) {
 			if (bytes_read) {
-				spin_unlock(&lr->wait.lock);
+				spin_unlock_irq(&lr->wait.lock);
 				return bytes_read;
 			}
 
 			if (file->f_flags & O_NONBLOCK) {
-				spin_unlock(&lr->wait.lock);
+				spin_unlock_irq(&lr->wait.lock);
 				return -EAGAIN;
 			}
 
-			ret = wait_event_interruptible_locked(lr->wait,
+			ret = wait_event_interruptible_locked_irq(lr->wait,
 					!kfifo_is_empty(&lr->events));
 			if (ret) {
-				spin_unlock(&lr->wait.lock);
+				spin_unlock_irq(&lr->wait.lock);
 				return ret;
 			}
 		}
 
 		ret = kfifo_out(&lr->events, &le, 1);
-		spin_unlock(&lr->wait.lock);
+		spin_unlock_irq(&lr->wait.lock);
 		if (ret != 1) {
 			/*
 			 * This should never happen - we were holding the
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH v2 2/2 gpiolib: cdev: Fix &le->wait.lock deadlock issue

The softirq lineevent_irq_thread() could deadlock on &le->wait.lock if it preempts lineevent_read_unlocked() which acquires the same lock but not disable irq.

Fix the potential deadlock by spin_lock_irq().

Fixes dea9c80ee672 ("gpiolib: rework the locking mechanism for lineevent kfifo")
Signed-off-by Chengfeng Ye <[email protected]>
---
 drivers/gpio/gpiolib-cdev.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/gpio/gpiolib-cdev.c +6 -6
index f768d46bdea7..a2c108f172f2 100644
@@ -1858,28 +1858,28 @@ static ssize_t lineevent_read_unlocked(struct file *file, char __user *buf,
 		return -EINVAL;
 
 	do {
-		spin_lock(&le->wait.lock);
+		spin_lock_irq(&le->wait.lock);
 		if (kfifo_is_empty(&le->events)) {
 			if (bytes_read) {
-				spin_unlock(&le->wait.lock);
+				spin_unlock_irq(&le->wait.lock);
 				return bytes_read;
 			}
 
 			if (file->f_flags & O_NONBLOCK) {
-				spin_unlock(&le->wait.lock);
+				spin_unlock_irq(&le->wait.lock);
 				return -EAGAIN;
 			}
 
-			ret = wait_event_interruptible_locked(le->wait,
+			ret = wait_event_interruptible_locked_irq(le->wait,
 					!kfifo_is_empty(&le->events));
 			if (ret) {
-				spin_unlock(&le->wait.lock);
+				spin_unlock_irq(&le->wait.lock);
 				return ret;
 			}
 		}
 
 		ret = kfifo_out(&le->events, &ge, 1);
-		spin_unlock(&le->wait.lock);
+		spin_unlock_irq(&le->wait.lock);
 		if (ret != 1) {
 			/*
 			 * This should never happen - we were holding the lock
Signature · 1 line
2.17.1