On kernels with the 4G/4G patch (like some of the recent kernels in
Fedora Core 2 development), writing stuff to the /proc/acpi/toshiba/*
files causes an oops. As it turns out, this is because the driver is
accessing userspace data without first doing copy_from_user(). IOW, this
is a bug in toshiba_acpi, not a bug in the 4G/4G patch.
Here's a patch to fix this bug. I've tested it on 2.6.4 + some patches
from the FC kernels (including the 4G/4G patch) and it fixes my oopses.
I have also tested it against vanilla 2.6.4 and I haven't encountered
any regressions.
If there are any problems with this patch, let me know.
-Barry K. Nathan <[email protected]>
diff -ruN linux-2.6.4/drivers/acpi/toshiba_acpi.c linux-2.6.4-bkn1/drivers/acpi/toshiba_acpi.c
--- linux-2.6.4/drivers/acpi/toshiba_acpi.c 2004-03-12 21:31:59.000000000 -0800
+++ linux-2.6.4-bkn1/drivers/acpi/toshiba_acpi.c 2004-03-12 22:27:07.000000000 -0800
@@ -41,6 +41,7 @@
#include <linux/init.h>
#include <linux/types.h>
#include <linux/proc_fs.h>
+#include <asm/uaccess.h>
#include <acpi/acpi_drivers.h>
@@ -269,10 +270,18 @@
}
static int
-dispatch_write(struct file* file, const char* buffer, unsigned long count,
- ProcItem* item)
+dispatch_write(struct file* file, const char __user *buffer,
+ unsigned long count, ProcItem* item)
{
- return item->write_func(buffer, count);
+ char str[48] = {'\0'};
+
+ if (count > sizeof(str) - 1)
+ return count;
+
+ if (copy_from_user(str, buffer, count))
+ return -EFAULT;
+
+ return item->write_func(str, count);
}
static char*
Thank you for tracking this down.
Please don't apply this patch to the kernel tree. I will submit a
variation via Len Brown. In any case, it appears at least one other
ACPI driver has a similar bug, so best to go through Len.
-John
Barry K. Nathan wrote:
> On kernels with the 4G/4G patch (like some of the recent kernels in
> Fedora Core 2 development), writing stuff to the /proc/acpi/toshiba/*
> files causes an oops. As it turns out, this is because the driver is
> accessing userspace data without first doing copy_from_user(). IOW, this
> is a bug in toshiba_acpi, not a bug in the 4G/4G patch.
>
> Here's a patch to fix this bug. I've tested it on 2.6.4 + some patches
> from the FC kernels (including the 4G/4G patch) and it fixes my oopses.
> I have also tested it against vanilla 2.6.4 and I haven't encountered
> any regressions.
>
> If there are any problems with this patch, let me know.
>
> -Barry K. Nathan <[email protected]>
>
>
> diff -ruN linux-2.6.4/drivers/acpi/toshiba_acpi.c linux-2.6.4-bkn1/drivers/acpi/toshiba_acpi.c
> --- linux-2.6.4/drivers/acpi/toshiba_acpi.c 2004-03-12 21:31:59.000000000 -0800
> +++ linux-2.6.4-bkn1/drivers/acpi/toshiba_acpi.c 2004-03-12 22:27:07.000000000 -0800
> @@ -41,6 +41,7 @@
> #include <linux/init.h>
> #include <linux/types.h>
> #include <linux/proc_fs.h>
> +#include <asm/uaccess.h>
>
> #include <acpi/acpi_drivers.h>
>
> @@ -269,10 +270,18 @@
> }
>
> static int
> -dispatch_write(struct file* file, const char* buffer, unsigned long count,
> - ProcItem* item)
> +dispatch_write(struct file* file, const char __user *buffer,
> + unsigned long count, ProcItem* item)
> {
> - return item->write_func(buffer, count);
> + char str[48] = {'\0'};
> +
> + if (count > sizeof(str) - 1)
> + return count;
> +
> + if (copy_from_user(str, buffer, count))
> + return -EFAULT;
> +
> + return item->write_func(str, count);
> }
>
> static char*
>
>
--
http:// if ile.org/
On Sun, Mar 14, 2004 at 12:52:04AM -0500, John Belmonte wrote:
> Thank you for tracking this down.
That reminds me of something I forgot to mention in my original message.
I filed this in Red Hat Bugzilla (#117682), and Arjan van de Ven
mentioned the likely cause. I then confirmed it and coded up my patch.
So, Arjan deserves credit too. I meant to mention this in my original
e-mail, but I accidentally forgot. Sorry about that.
> Please don't apply this patch to the kernel tree. I will submit a
> variation via Len Brown. In any case, it appears at least one other
> ACPI driver has a similar bug, so best to go through Len.
Ok, sounds good to me.
-Barry K. Nathan <[email protected]>