| 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
| ||
|
Message-ID: <20240905041732.2034348-24-dmitry.torokhov@gmail.com>
Date: Wed, 4 Sep 2024 21:17:28 -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 23/24] Input: userio - switch to using cleanup functions
Use __free() and guard() primitives to simplify the code and error
handling.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
---
drivers/input/serio/userio.c | 141 +++++++++++++++++------------------
1 file changed, 70 insertions(+), 71 deletions(-)
diff --git a/drivers/input/serio/userio.c b/drivers/input/serio/userio.c
index a88e2eee55c3..66c9838a1fa7 100644
--- a/drivers/input/serio/userio.c
+++ b/drivers/input/serio/userio.c
@@ -55,18 +55,15 @@ struct userio_device {
static int userio_device_write(struct serio *id, unsigned char val)
{
struct userio_device *userio = id->port_data;
- unsigned long flags;
- spin_lock_irqsave(&userio->buf_lock, flags);
+ scoped_guard(spinlock_irqsave, &userio->buf_lock) {
+ userio->buf[userio->head] = val;
+ userio->head = (userio->head + 1) % USERIO_BUFSIZE;
- userio->buf[userio->head] = val;
- userio->head = (userio->head + 1) % USERIO_BUFSIZE;
-
- if (userio->head == userio->tail)
- dev_warn(userio_misc.this_device,
- "Buffer overflowed, userio client isn't keeping up");
-
- spin_unlock_irqrestore(&userio->buf_lock, flags);
+ if (userio->head == userio->tail)
+ dev_warn(userio_misc.this_device,
+ "Buffer overflowed, userio client isn't keeping up");
+ }
wake_up_interruptible(&userio->waitq);
@@ -75,9 +72,8 @@ static int userio_device_write(struct serio *id, unsigned char val)
static int userio_char_open(struct inode *inode, struct file *file)
{
- struct userio_device *userio;
-
- userio = kzalloc(sizeof(*userio), GFP_KERNEL);
+ struct userio_device *userio __free(kfree) =
+ kzalloc(sizeof(*userio), GFP_KERNEL);
if (!userio)
return -ENOMEM;
@@ -86,15 +82,13 @@ static int userio_char_open(struct inode *inode, struct file *file)
init_waitqueue_head(&userio->waitq);
userio->serio = kzalloc(sizeof(*userio->serio), GFP_KERNEL);
- if (!userio->serio) {
- kfree(userio);
+ if (!userio->serio)
return -ENOMEM;
- }
userio->serio->write = userio_device_write;
- userio->serio->port_data = userio;
+ userio->serio->port_data = userio;;
- file->private_data = userio;
+ file->private_data = no_free_ptr(userio);
return 0;
}
@@ -118,14 +112,32 @@ static int userio_char_release(struct inode *inode, struct file *file)
return 0;
}
+static size_t userio_fetch_data(struct userio_device *userio, u8 *buf,
+ size_t count, size_t *copylen)
+{
+ size_t available, len;
+
+ guard(spinlock_irqsave)(&userio->buf_lock);
+
+ available = CIRC_CNT_TO_END(userio->head, userio->tail,
+ USERIO_BUFSIZE);
+ len = min(available, count);
+ if (len) {
+ memcpy(buf, &userio->buf[userio->tail], len);
+ userio->tail = (userio->tail + len) % USERIO_BUFSIZE;
+ }
+
+ *copylen = len;
+ return available;
+}
+
static ssize_t userio_char_read(struct file *file, char __user *user_buffer,
size_t count, loff_t *ppos)
{
struct userio_device *userio = file->private_data;
int error;
- size_t nonwrap_len, copylen;
- unsigned char buf[USERIO_BUFSIZE];
- unsigned long flags;
+ size_t available, copylen;
+ u8 buf[USERIO_BUFSIZE];
/*
* By the time we get here, the data that was waiting might have
@@ -135,21 +147,8 @@ static ssize_t userio_char_read(struct file *file, char __user *user_buffer,
* of course).
*/
for (;;) {
- spin_lock_irqsave(&userio->buf_lock, flags);
-
- nonwrap_len = CIRC_CNT_TO_END(userio->head,
- userio->tail,
- USERIO_BUFSIZE);
- copylen = min(nonwrap_len, count);
- if (copylen) {
- memcpy(buf, &userio->buf[userio->tail], copylen);
- userio->tail = (userio->tail + copylen) %
- USERIO_BUFSIZE;
- }
-
- spin_unlock_irqrestore(&userio->buf_lock, flags);
-
- if (nonwrap_len)
+ available = userio_fetch_data(userio, buf, count, ©len);
+ if (available)
break;
/* buffer was/is empty */
@@ -176,40 +175,21 @@ static ssize_t userio_char_read(struct file *file, char __user *user_buffer,
return copylen;
}
-static ssize_t userio_char_write(struct file *file, const char __user *buffer,
- size_t count, loff_t *ppos)
+static int userio_execute_cmd(struct userio_device *userio,
+ const struct userio_cmd *cmd)
{
- struct userio_device *userio = file->private_data;
- struct userio_cmd cmd;
- int error;
-
- if (count != sizeof(cmd)) {
- dev_warn(userio_misc.this_device, "Invalid payload size\n");
- return -EINVAL;
- }
-
- if (copy_from_user(&cmd, buffer, sizeof(cmd)))
- return -EFAULT;
-
- error = mutex_lock_interruptible(&userio->mutex);
- if (error)
- return error;
-
- switch (cmd.type) {
+ switch (cmd->type) {
case USERIO_CMD_REGISTER:
if (!userio->serio->id.type) {
dev_warn(userio_misc.this_device,
"No port type given on /dev/userio\n");
-
- error = -EINVAL;
- goto out;
+ return -EINVAL;
}
if (userio->running) {
dev_warn(userio_misc.this_device,
"Begin command sent, but we're already running\n");
- error = -EBUSY;
- goto out;
+ return -EBUSY;
}
userio->running = true;
@@ -220,32 +200,51 @@ static ssize_t userio_char_write(struct file *file, const char __user *buffer,
if (userio->running) {
dev_warn(userio_misc.this_device,
"Can't change port type on an already running userio instance\n");
- error = -EBUSY;
- goto out;
+ return -EBUSY;
}
- userio->serio->id.type = cmd.data;
+ userio->serio->id.type = cmd->data;
break;
case USERIO_CMD_SEND_INTERRUPT:
if (!userio->running) {
dev_warn(userio_misc.this_device,
"The device must be registered before sending interrupts\n");
- error = -ENODEV;
- goto out;
+ return -ENODEV;
}
- serio_interrupt(userio->serio, cmd.data, 0);
+ serio_interrupt(userio->serio, cmd->data, 0);
break;
default:
- error = -EOPNOTSUPP;
- goto out;
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static ssize_t userio_char_write(struct file *file, const char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct userio_device *userio = file->private_data;
+ struct userio_cmd cmd;
+ int error;
+
+ if (count != sizeof(cmd)) {
+ dev_warn(userio_misc.this_device, "Invalid payload size\n");
+ return -EINVAL;
+ }
+
+ if (copy_from_user(&cmd, buffer, sizeof(cmd)))
+ return -EFAULT;
+
+ scoped_cond_guard(mutex_intr, return -EINTR, &userio->mutex) {
+ error = userio_execute_cmd(userio, &cmd);
+ if (error)
+ return error;
}
-out:
- mutex_unlock(&userio->mutex);
- return error ?: count;
+ return count;
}
static __poll_t userio_char_poll(struct file *file, poll_table *wait)
--
2.46.0.469.g59c65b2a67-goog
Powered by blists - more mailing lists