linux-kernel - [PATCH] acpi-video: Fix integer overflow and possible kernel stack trashing

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <200907191336.38261.mb@bu3sch.de>
Date:	Sun, 19 Jul 2009 13:36:38 +0200
From:	Michael Buesch <mb@...sch.de>
To:	linux-kernel@...r.kernel.org
Cc:	lenb@...nel.org, linux-acpi@...r.kernel.org
Subject: [PATCH] acpi-video: Fix integer overflow and possible kernel stack trashing

This patch fixes a possible kernel crash through stack trashing triggered
by an integer overflow. If count passed from userspace is (size_t)-1lu, the
range check will overflow and return false. So the copy_from_user() will
end up attempting to copy 0xFFFFFFFF (or 0xFFFFFFFFFFFFFFFF) bytes to the kernel stack.
Of course the copy will fail at some point, because we can't allocate a buffer that big.
But it will copy as much as it can and then return with an -EFAULT.
This means the userspace process writing to this proc file controls
the kernel stack.

This is probably not useable for a privilege escalation, because the proc file
has permissions (S_IFREG | S_IRUGO | S_IWUSR). So only root will be able to crash the machine.

Signed-off-by: Michael Buesch <mb@...sch.de>
Cc: stable@...nel.org

---

This patch is completely untested, because I do not have a machine with acpi-video.

---
 drivers/acpi/video.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-2.6.orig/drivers/acpi/video.c
+++ linux-2.6/drivers/acpi/video.c
@@ -1185,21 +1185,21 @@ acpi_video_device_write_state(struct fil
 			      const char __user * buffer,
 			      size_t count, loff_t * data)
 {
 	int status;
 	struct seq_file *m = file->private_data;
 	struct acpi_video_device *dev = m->private;
 	char str[12] = { 0 };
 	u32 state = 0;

-	if (!dev || count + 1 > sizeof str)
+	if (!dev || count >= sizeof str)
 		return -EINVAL;

 	if (copy_from_user(str, buffer, count))
 		return -EFAULT;

 	str[count] = 0;
 	state = simple_strtoul(str, NULL, 0);
 	state &= ((1ul << 31) | (1ul << 30) | (1ul << 0));

 	status = acpi_video_device_set_state(dev, state);

-- 
Greetings, Michael.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/