lkml.org 
[lkml]   [2023]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] ACPI: Make custom_method use per-open state
Date
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@gmail.com>
Cc: Swift Geek <swiftgeek@gmail.com>
Signed-off-by: Pedro Falcato <pedro.falcato@gmail.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
\
 
 \ /
  Last update: 2023-03-27 00:04    [W:0.279 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site