| 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: <20230131233755.58942-1-pedro.falcato@gmail.com>
Date: Tue, 31 Jan 2023 23:37:55 +0000
From: Pedro Falcato <pedro.falcato@...il.com>
To: linux-acpi@...r.kernel.org
Cc: "Rafael J . Wysocki" <rafael@...nel.org>,
Len Brown <lenb@...nel.org>, linux-kernel@...r.kernel.org,
rui.zhang@...el.com, Pedro Falcato <pedro.falcato@...il.com>,
Hang Zhang <zh.nvgt@...il.com>,
Swift Geek <swiftgeek@...il.com>
Subject: [PATCH] ACPI: Make custom_method use per-open state
Make custom_method keep its own per-file-open state instead of global
state in order to avoid race conditions[1] and other possible conflicts
with other concurrent users.
Link: https://lore.kernel.org/linux-acpi/20221227063335.61474-1-zh.nvgt@gmail.com/ # [1]
Reported-by: Hang Zhang <zh.nvgt@...il.com>
Cc: Swift Geek <swiftgeek@...il.com>
Signed-off-by: Pedro Falcato <pedro.falcato@...il.com>
---
This patch addresses Hang's problems plus the ones raised by Rafael in his review (see link above).
https://lore.kernel.org/lkml/2667007.mvXUDI8C0e@kreacher/ was submitted but since there were still people
that wanted this feature, I took my time to write up a patch that should fix the issues.
Hopefully the linux-acpi maintainers have not decided to remove custom_method just yet.
drivers/acpi/custom_method.c | 119 +++++++++++++++++++++++++++--------
1 file changed, 92 insertions(+), 27 deletions(-)
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index d39a9b47472..034fb14f118 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -17,73 +17,138 @@ MODULE_LICENSE("GPL");
static struct dentry *cm_dentry;
+struct custom_method_state {
+ char *buf;
+ u32 max_size;
+ u32 uncopied_bytes;
+ struct mutex lock;
+};
+
+static int cm_open(struct inode *inode, struct file *file)
+{
+ struct custom_method_state *state;
+
+ state = kzalloc(sizeof(struct custom_method_state), GFP_KERNEL);
+
+ if (!state)
+ return -ENOMEM;
+
+ file->private_data = state;
+ mutex_init(&state->lock);
+
+ return 0;
+}
+
+static int cm_release(struct inode *inode, struct file *file)
+{
+ struct custom_method_state *state;
+
+ state = file->private_data;
+
+ mutex_destroy(&state->lock);
+
+ /* Make sure the buf gets freed */
+ kfree(state->buf);
+
+ kfree(state);
+ return 0;
+}
+
/* /sys/kernel/debug/acpi/custom_method */
static ssize_t cm_write(struct file *file, const char __user *user_buf,
size_t count, loff_t *ppos)
{
- static char *buf;
- static u32 max_size;
- static u32 uncopied_bytes;
+ struct custom_method_state *state;
+ char *buf;
struct acpi_table_header table;
acpi_status status;
int ret;
+ state = file->private_data;
+ buf = state->buf;
+
ret = security_locked_down(LOCKDOWN_ACPI_TABLES);
if (ret)
return ret;
+ mutex_lock(&state->lock);
+
if (!(*ppos)) {
/* parse the table header to get the table length */
- if (count <= sizeof(struct acpi_table_header))
- return -EINVAL;
+ if (count <= sizeof(struct acpi_table_header)) {
+ count = -EINVAL;
+ goto out;
+ }
+
if (copy_from_user(&table, user_buf,
- sizeof(struct acpi_table_header)))
- return -EFAULT;
- uncopied_bytes = max_size = table.length;
+ sizeof(struct acpi_table_header))) {
+ count = -EFAULT;
+ goto out;
+ }
+
+ state->uncopied_bytes = state->max_size = table.length;
/* make sure the buf is not allocated */
kfree(buf);
- buf = kzalloc(max_size, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
+ buf = state->buf = kzalloc(state->max_size, GFP_KERNEL);
+ if (!buf) {
+ count = -ENOMEM;
+ goto out;
+ }
}
- if (buf == NULL)
- return -EINVAL;
+ /* Check if someone seeked ahead or if we errored out
+ * (buf will be NULL)
+ */
+ if (buf == NULL) {
+ count = -EINVAL;
+ goto out;
+ }
- if ((*ppos > max_size) ||
- (*ppos + count > max_size) ||
+ if ((*ppos > state->max_size) ||
+ (*ppos + count > state->max_size) ||
(*ppos + count < count) ||
- (count > uncopied_bytes)) {
- kfree(buf);
- buf = NULL;
- return -EINVAL;
+ (count > state->uncopied_bytes)) {
+ count = -EINVAL;
+ goto err_free;
}
if (copy_from_user(buf + (*ppos), user_buf, count)) {
- kfree(buf);
- buf = NULL;
- return -EFAULT;
+ count = -EFAULT;
+ goto err_free;
}
- uncopied_bytes -= count;
+ state->uncopied_bytes -= count;
*ppos += count;
- if (!uncopied_bytes) {
+ if (!state->uncopied_bytes) {
status = acpi_install_method(buf);
kfree(buf);
- buf = NULL;
- if (ACPI_FAILURE(status))
- return -EINVAL;
+ state->buf = NULL;
+
+ if (ACPI_FAILURE(status)) {
+ count = -EINVAL;
+ goto out;
+ }
+
add_taint(TAINT_OVERRIDDEN_ACPI_TABLE, LOCKDEP_NOW_UNRELIABLE);
}
+out:
+ mutex_unlock(&state->lock);
+ return count;
+err_free:
+ mutex_unlock(&state->lock);
+ kfree(buf);
+ state->buf = NULL;
return count;
}
static const struct file_operations cm_fops = {
.write = cm_write,
+ .open = cm_open,
+ .release = cm_release,
.llseek = default_llseek,
};
--
2.39.0
Powered by blists - more mailing lists