From: Kees Cook keescook@chromium.org
This reverts commit 03d1571d9513369c17e6848476763ebbd10ec2cb.
While /sys/kernel/debug/acpi/custom_method is already a privileged-only API providing proxied arbitrary write access to kernel memory[1][2], with existing race conditions[3] in buffer allocation and use that could lead to memory leaks and use-after-free conditions, the above commit appears to accidentally make the use-after-free conditions even easier to accomplish. ("buf" is a global variable and prior kfree()s would set buf back to NULL.)
This entire interface needs to be reworked (if not entirely removed).
[1] https://lore.kernel.org/lkml/20110222193250.GA23913@outflux.net/ [2] https://lore.kernel.org/lkml/201906221659.B618D83@keescook/ [3] https://lore.kernel.org/lkml/20170109231323.GA89642@beast/
Cc: Wenwen Wang wenwen@cs.uga.edu Fixes: 03d1571d9513 ("ACPI: custom_method: fix memory leaks") Cc: stable stable@vger.kernel.org Signed-off-by: Kees Cook keescook@chromium.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/acpi/custom_method.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c index 443fdf62dd22..72469a49837d 100644 --- a/drivers/acpi/custom_method.c +++ b/drivers/acpi/custom_method.c @@ -53,10 +53,8 @@ static ssize_t cm_write(struct file *file, const char __user *user_buf, if ((*ppos > max_size) || (*ppos + count > max_size) || (*ppos + count < count) || - (count > uncopied_bytes)) { - kfree(buf); + (count > uncopied_bytes)) return -EINVAL; - }
if (copy_from_user(buf + (*ppos), user_buf, count)) { kfree(buf); @@ -76,7 +74,6 @@ static ssize_t cm_write(struct file *file, const char __user *user_buf, add_taint(TAINT_OVERRIDDEN_ACPI_TABLE, LOCKDEP_NOW_UNRELIABLE); }
- kfree(buf); return count; }