From: Michael Buesch <mb@xxxxxxxxx>
Avoid heap trashing triggered by an integer overflow of the
userspace controlled "count" variable.
If userspace passes in a "count" of (size_t)-1l, the kmalloc size will
overflow to ((size_t)-1l + 1) = 0. If kmalloc() is called with zero size,
it will return the ZERO_SIZE_PTR, which is (void *)16.
This will pass the !tmp_buffer sanity check.
After that, copy_from_user() will attempt to copy 0xFFFFFFFF
(or 0xFFFFFFFFFFFFFFFF on 64bit) bytes to (void *)16, which is within
the NULL page.
A possible testcase could look like this:
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <string.h>
int main(int argc, char **argv)
{
int fd;
char *buf;
if (argc != 2) {
printf("Usage: %s /proc/acpi/toshiba/filename\n", argv[0]);
return 1;
}
fd = open(argv[1], O_RDWR);
if (fd < 0) {
printf("Could not open proc file\n");
return 1;
}
buf = malloc(1337);
if (!buf) {
printf("Out of memory\n");
return 1;
}
memset(buf, 0x66, 1337);
write(fd, buf, (size_t)-1l); /* boom!! */
}
We avoid the integer overrun by putting an arbitrary limit on the count.
PAGE_SIZE sounds like a sane limit.
Signed-off-by: Michael Buesch <mb@xxxxxxxxx>
Cc: stable@xxxxxxxxxx
---
This patch is completely untested due to lack of supported device.
The proc file is only writeable by root in the default setup,
so it's probably not exploitable as-is.
The toshiba-acpi driver is orphaned, so I hope somebody will pick this up.
---
drivers/platform/x86/toshiba_acpi.c | 2 ++
1 file changed, 2 insertions(+)
--- linux-2.6.orig/drivers/platform/x86/toshiba_acpi.c
+++ linux-2.6/drivers/platform/x86/toshiba_acpi.c
@@ -388,20 +388,22 @@ dispatch_read(char *page, char **start,
return len;
}
static int
dispatch_write(struct file *file, const char __user * buffer,
unsigned long count, ProcItem * item)
{
int result;
char *tmp_buffer;
+ if (count > PAGE_SIZE - 1)
+ return -EINVAL;
/* Arg buffer points to userspace memory, which can't be accessed
* directly. Since we're making a copy, zero-terminate the
* destination so that sscanf can be used on it safely.
*/
tmp_buffer = kmalloc(count + 1, GFP_KERNEL);
if (!tmp_buffer)
return -ENOMEM;
if (copy_from_user(tmp_buffer, buffer, count)) {
result = -EFAULT;
--
Greetings, Michael.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
