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-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240905041732.2034348-21-dmitry.torokhov@gmail.com>
Date: Wed,  4 Sep 2024 21:17:25 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: linux-input@...r.kernel.org
Cc: Pali Rohár <pali@...nel.org>,
	Helge Deller <deller@....de>,
	"K. Y. Srinivasan" <kys@...rosoft.com>,
	Wei Liu <wei.liu@...nel.org>,
	Dexuan Cui <decui@...rosoft.com>,
	Samuel Holland <samuel@...lland.org>,
	Lyude Paul <thatslyude@...il.com>,
	Michal Simek <michal.simek@....com>,
	Hans de Goede <hdegoede@...hat.com>,
	linux-kernel@...r.kernel.org,
	linux-hyperv@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-sunxi@...ts.linux.dev
Subject: [PATCH 20/24] Input: serio_raw - use guard notation for locks and other resources

Use guard notation when acquiring mutexes and spinlocks, and when
pausing and resuming serio port. Such guard notation makes the code
more compact and error handling more robust by ensuring that locks
are released in all code paths when control leaves critical section.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
---
 drivers/input/serio/serio_raw.c | 121 +++++++++++++-------------------
 1 file changed, 49 insertions(+), 72 deletions(-)

diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
index 0186d1b38f49..aef8301313b2 100644
--- a/drivers/input/serio/serio_raw.c
+++ b/drivers/input/serio/serio_raw.c
@@ -75,41 +75,31 @@ static int serio_raw_open(struct inode *inode, struct file *file)
 {
 	struct serio_raw *serio_raw;
 	struct serio_raw_client *client;
-	int retval;
 
-	retval = mutex_lock_interruptible(&serio_raw_mutex);
-	if (retval)
-		return retval;
+	scoped_guard(mutex_intr, &serio_raw_mutex) {
+		serio_raw = serio_raw_locate(iminor(inode));
+		if (!serio_raw)
+			return -ENODEV;
 
-	serio_raw = serio_raw_locate(iminor(inode));
-	if (!serio_raw) {
-		retval = -ENODEV;
-		goto out;
-	}
+		if (serio_raw->dead)
+			return -ENODEV;
 
-	if (serio_raw->dead) {
-		retval = -ENODEV;
-		goto out;
-	}
+		client = kzalloc(sizeof(*client), GFP_KERNEL);
+		if (!client)
+			return -ENOMEM;
 
-	client = kzalloc(sizeof(*client), GFP_KERNEL);
-	if (!client) {
-		retval = -ENOMEM;
-		goto out;
-	}
+		client->serio_raw = serio_raw;
+		file->private_data = client;
 
-	client->serio_raw = serio_raw;
-	file->private_data = client;
+		kref_get(&serio_raw->kref);
 
-	kref_get(&serio_raw->kref);
+		scoped_guard(serio_pause_rx, serio_raw->serio)
+			list_add_tail(&client->node, &serio_raw->client_list);
 
-	serio_pause_rx(serio_raw->serio);
-	list_add_tail(&client->node, &serio_raw->client_list);
-	serio_continue_rx(serio_raw->serio);
+		return 0;
+	}
 
-out:
-	mutex_unlock(&serio_raw_mutex);
-	return retval;
+	return -EINTR;
 }
 
 static void serio_raw_free(struct kref *kref)
@@ -126,9 +116,8 @@ static int serio_raw_release(struct inode *inode, struct file *file)
 	struct serio_raw_client *client = file->private_data;
 	struct serio_raw *serio_raw = client->serio_raw;
 
-	serio_pause_rx(serio_raw->serio);
-	list_del(&client->node);
-	serio_continue_rx(serio_raw->serio);
+	scoped_guard(serio_pause_rx, serio_raw->serio)
+		list_del(&client->node);
 
 	kfree(client);
 
@@ -139,19 +128,15 @@ static int serio_raw_release(struct inode *inode, struct file *file)
 
 static bool serio_raw_fetch_byte(struct serio_raw *serio_raw, char *c)
 {
-	bool empty;
+	guard(serio_pause_rx)(serio_raw->serio);
 
-	serio_pause_rx(serio_raw->serio);
-
-	empty = serio_raw->head == serio_raw->tail;
-	if (!empty) {
-		*c = serio_raw->queue[serio_raw->tail];
-		serio_raw->tail = (serio_raw->tail + 1) % SERIO_RAW_QUEUE_LEN;
-	}
+	if (serio_raw->head == serio_raw->tail)
+		return false; /* queue is empty */
 
-	serio_continue_rx(serio_raw->serio);
+	*c = serio_raw->queue[serio_raw->tail];
+	serio_raw->tail = (serio_raw->tail + 1) % SERIO_RAW_QUEUE_LEN;
 
-	return !empty;
+	return true;
 }
 
 static ssize_t serio_raw_read(struct file *file, char __user *buffer,
@@ -200,40 +185,32 @@ static ssize_t serio_raw_write(struct file *file, const char __user *buffer,
 {
 	struct serio_raw_client *client = file->private_data;
 	struct serio_raw *serio_raw = client->serio_raw;
-	int retval = 0;
+	int written;
 	unsigned char c;
 
-	retval = mutex_lock_interruptible(&serio_raw_mutex);
-	if (retval)
-		return retval;
+	scoped_guard(mutex_intr, &serio_raw_mutex) {
+		if (serio_raw->dead)
+			return -ENODEV;
 
-	if (serio_raw->dead) {
-		retval = -ENODEV;
-		goto out;
-	}
+		if (count > 32)
+			count = 32;
 
-	if (count > 32)
-		count = 32;
+		while (count--) {
+			if (get_user(c, buffer++))
+				return -EFAULT;
 
-	while (count--) {
-		if (get_user(c, buffer++)) {
-			retval = -EFAULT;
-			goto out;
-		}
+			if (serio_write(serio_raw->serio, c)) {
+				/* Either signal error or partial write */
+				return written ?: -EIO;
+			}
 
-		if (serio_write(serio_raw->serio, c)) {
-			/* Either signal error or partial write */
-			if (retval == 0)
-				retval = -EIO;
-			goto out;
+			written++;
 		}
 
-		retval++;
+		return written;
 	}
 
-out:
-	mutex_unlock(&serio_raw_mutex);
-	return retval;
+	return -EINTR;
 }
 
 static __poll_t serio_raw_poll(struct file *file, poll_table *wait)
@@ -379,10 +356,10 @@ static void serio_raw_hangup(struct serio_raw *serio_raw)
 {
 	struct serio_raw_client *client;
 
-	serio_pause_rx(serio_raw->serio);
-	list_for_each_entry(client, &serio_raw->client_list, node)
-		kill_fasync(&client->fasync, SIGIO, POLL_HUP);
-	serio_continue_rx(serio_raw->serio);
+	scoped_guard(serio_pause_rx, serio_raw->serio) {
+		list_for_each_entry(client, &serio_raw->client_list, node)
+			kill_fasync(&client->fasync, SIGIO, POLL_HUP);
+	}
 
 	wake_up_interruptible(&serio_raw->wait);
 }
@@ -394,10 +371,10 @@ static void serio_raw_disconnect(struct serio *serio)
 
 	misc_deregister(&serio_raw->dev);
 
-	mutex_lock(&serio_raw_mutex);
-	serio_raw->dead = true;
-	list_del_init(&serio_raw->node);
-	mutex_unlock(&serio_raw_mutex);
+	scoped_guard(mutex, &serio_raw_mutex) {
+		serio_raw->dead = true;
+		list_del_init(&serio_raw->node);
+	}
 
 	serio_raw_hangup(serio_raw);
 
-- 
2.46.0.469.g59c65b2a67-goog


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ