[PATCH]: MTRR: fix 32-bit ioctls on x64_32
Giuliano Procida
Signed-off-by: Giuliano Procida <giuliano...@googlemail.com>
---
Fixed incomplete support for 32-bit compatibility ioctls in
2.6.19.1. They were unhandled in one of three case-statements.
Testing using X server before and after change.
--- linux-source-2.6.19.1.orig/arch/i386/kernel/cpu/mtrr/if.c 2006-12-11 19:32:53.000000000 +0000
+++ linux-source-2.6.19.1/arch/i386/kernel/cpu/mtrr/if.c 2007-01-16 07:31:06.000000000 +0000
@@ -211,6 +211,9 @@ mtrr_ioctl(struct file *file, unsigned i
default:
return -ENOTTY;
case MTRRIOC_ADD_ENTRY:
+#ifdef CONFIG_COMPAT
+ case MTRRIOC32_ADD_ENTRY:
+#endif
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
err =
@@ -218,21 +221,33 @@ mtrr_ioctl(struct file *file, unsigned i
file, 0);
break;
case MTRRIOC_SET_ENTRY:
+#ifdef CONFIG_COMPAT
+ case MTRRIOC32_SET_ENTRY:
+#endif
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
err = mtrr_add(sentry.base, sentry.size, sentry.type, 0);
break;
case MTRRIOC_DEL_ENTRY:
+#ifdef CONFIG_COMPAT
+ case MTRRIOC32_DEL_ENTRY:
+#endif
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
err = mtrr_file_del(sentry.base, sentry.size, file, 0);
break;
case MTRRIOC_KILL_ENTRY:
+#ifdef CONFIG_COMPAT
+ case MTRRIOC32_KILL_ENTRY:
+#endif
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
err = mtrr_del(-1, sentry.base, sentry.size);
break;
case MTRRIOC_GET_ENTRY:
+#ifdef CONFIG_COMPAT
+ case MTRRIOC32_GET_ENTRY:
+#endif
if (gentry.regnum >= num_var_ranges)
return -EINVAL;
mtrr_if->get(gentry.regnum, &gentry.base, &gentry.size, &type);
@@ -249,6 +264,9 @@ mtrr_ioctl(struct file *file, unsigned i
break;
case MTRRIOC_ADD_PAGE_ENTRY:
+#ifdef CONFIG_COMPAT
+ case MTRRIOC32_ADD_PAGE_ENTRY:
+#endif
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
err =
@@ -256,21 +274,33 @@ mtrr_ioctl(struct file *file, unsigned i
file, 1);
break;
case MTRRIOC_SET_PAGE_ENTRY:
+#ifdef CONFIG_COMPAT
+ case MTRRIOC32_SET_PAGE_ENTRY:
+#endif
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
err = mtrr_add_page(sentry.base, sentry.size, sentry.type, 0);
break;
case MTRRIOC_DEL_PAGE_ENTRY:
+#ifdef CONFIG_COMPAT
+ case MTRRIOC32_DEL_PAGE_ENTRY:
+#endif
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
err = mtrr_file_del(sentry.base, sentry.size, file, 1);
break;
case MTRRIOC_KILL_PAGE_ENTRY:
+#ifdef CONFIG_COMPAT
+ case MTRRIOC32_KILL_PAGE_ENTRY:
+#endif
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
err = mtrr_del_page(-1, sentry.base, sentry.size);
break;
case MTRRIOC_GET_PAGE_ENTRY:
+#ifdef CONFIG_COMPAT
+ case MTRRIOC32_GET_PAGE_ENTRY:
+#endif
if (gentry.regnum >= num_var_ranges)
return -EINVAL;
mtrr_if->get(gentry.regnum, &gentry.base, &gentry.size, &type);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Mikael Pettersson
etc
These #ifdefs are too ugly.
Since you apparently just add aliases for the case labels,
and do no actual code changes, why not
1. make the new cases unconditional, or
2. invoke a translation function before the switch which
maps the MTRRCIOC32_ constants to what the kernel uses
/Mikael
Giuliano Procida
On 16/01/07, Mikael Pettersson <mi...@it.uu.se> wrote:
> On Tue, 16 Jan 2007 08:14:30 +0000, Giuliano Procida wrote:
> These #ifdefs are too ugly.
Agreed that the #ifdefs are rather ugly, but they were the smallest change.
Whoever wrote the original compat changes was relying on the IOC
constants being different for 32- and 64-bit userspace. This allowed the
lazy reuse of the whole ioctl function rather than having to write a complete
replacement compat_ioctl for fops.
> Since you apparently just add aliases for the case labels,
> and do no actual code changes, why not
> 1. make the new cases unconditional, or
The constants are only visible under CONFIG_COMPAT. I think they
should stay that way.
> 2. invoke a translation function before the switch which
> maps the MTRRCIOC32_ constants to what the kernel uses
Other things I considered:
3. write a wrapper compat function that calls the original, make the
original pure 64-bit
4. macroise the case labels away somehow
5. update cmd in place (cannot do this as it re-used in the third switch)
6. add real_cmd and switch on that instead (your 2.),
requires yet another switch and #ifdef.
It might be nicer to decode the IOC constants and use action, size and R/W flags
to control all the switches. Not sure myself.
Giuliano.
Giuliano Procida
> Adding a case can add substantially to the generated code, especially if
> it makes a compact set of case labels non-compact.
Is this one any better? It certainly makes for a slimmer object.
Compiled, but not yet tested. Caveat patcher.
Signed-off-by: Giuliano Procida <giuliano...@googlemail.com>
Giuliano.
--- linux-source-2.6.19.1.orig/arch/i386/kernel/cpu/mtrr/if.c 2006-12-11
19:32:53.000000000 +0000
+++ linux-source-2.6.19.1/arch/i386/kernel/cpu/mtrr/if.c 2007-01-22
23:34:48.000000000 +0000
@@ -154,150 +154,166 @@
mtrr_ioctl(struct file *file, unsigned int cmd, unsigned long __arg)
{
int err = 0;
+ unsigned ioctl_dir;
+ unsigned ioctl_nr;
+ unsigned ioctl_size;
mtrr_type type;
- struct mtrr_sentry sentry;
- struct mtrr_gentry gentry;
+ union mtrr_data {
+ struct mtrr_sentry sentry;
+ struct mtrr_gentry gentry;
+#ifdef CONFIG_COMPAT
+ struct mtrr_sentry32 sentry32;
+ struct mtrr_gentry32 gentry32;
+#endif
+ };
+ union mtrr_data u;
void __user *arg = (void __user *) __arg;
- switch (cmd) {
- case MTRRIOC_ADD_ENTRY:
- case MTRRIOC_SET_ENTRY:
- case MTRRIOC_DEL_ENTRY:
- case MTRRIOC_KILL_ENTRY:
- case MTRRIOC_ADD_PAGE_ENTRY:
- case MTRRIOC_SET_PAGE_ENTRY:
- case MTRRIOC_DEL_PAGE_ENTRY:
- case MTRRIOC_KILL_PAGE_ENTRY:
- if (copy_from_user(&sentry, arg, sizeof sentry))
- return -EFAULT;
- break;
- case MTRRIOC_GET_ENTRY:
- case MTRRIOC_GET_PAGE_ENTRY:
- if (copy_from_user(&gentry, arg, sizeof gentry))
- return -EFAULT;
+ /* check type and max size */
+ ioctl_size = _IOC_SIZE(cmd);
+ if (_IOC_TYPE(cmd) != MTRR_IOCTL_BASE || ioctl_size > sizeof(u))
+ return -ENOTTY;
+
+ /* copy from user */
+ ioctl_dir = _IOC_DIR(cmd);
+ if (ioctl_dir & _IOC_WRITE && copy_from_user(&u, arg, ioctl_size))
+ return -EFAULT;
+
+ /* check number, direction, size and permission */
+ ioctl_nr = _IOC_NR(cmd);
+ ioctl_size = _IOC_SIZE(cmd);
+ switch (_IOC_NR(cmd)) {
+ case _IOC_NR(MTRRIOC_ADD_ENTRY):
+ case _IOC_NR(MTRRIOC_SET_ENTRY):
+ case _IOC_NR(MTRRIOC_DEL_ENTRY):
+ case _IOC_NR(MTRRIOC_KILL_ENTRY):
+ case _IOC_NR(MTRRIOC_ADD_PAGE_ENTRY):
+ case _IOC_NR(MTRRIOC_SET_PAGE_ENTRY):
+ case _IOC_NR(MTRRIOC_DEL_PAGE_ENTRY):
+ case _IOC_NR(MTRRIOC_KILL_PAGE_ENTRY):
+ if (_IOC_DIR(cmd) != _IOC_WRITE)
+ return -ENOTTY;
+ switch (ioctl_size) {
+ case sizeof(struct mtrr_sentry):
break;
#ifdef CONFIG_COMPAT
- case MTRRIOC32_ADD_ENTRY:
- case MTRRIOC32_SET_ENTRY:
- case MTRRIOC32_DEL_ENTRY:
- case MTRRIOC32_KILL_ENTRY:
- case MTRRIOC32_ADD_PAGE_ENTRY:
- case MTRRIOC32_SET_PAGE_ENTRY:
- case MTRRIOC32_DEL_PAGE_ENTRY:
- case MTRRIOC32_KILL_PAGE_ENTRY: {
- struct mtrr_sentry32 __user *s32 = (struct mtrr_sentry32 __user *)__arg;
- err = get_user(sentry.base, &s32->base);
- err |= get_user(sentry.size, &s32->size);
- err |= get_user(sentry.type, &s32->type);
- if (err)
- return err;
- break;
- }
- case MTRRIOC32_GET_ENTRY:
- case MTRRIOC32_GET_PAGE_ENTRY: {
- struct mtrr_gentry32 __user *g32 = (struct mtrr_gentry32 __user *)__arg;
- err = get_user(gentry.regnum, &g32->regnum);
- err |= get_user(gentry.base, &g32->base);
- err |= get_user(gentry.size, &g32->size);
- err |= get_user(gentry.type, &g32->type);
- if (err)
- return err;
+ case sizeof(struct mtrr_sentry32):
+ {
+ struct mtrr_sentry32 s32 = u.sentry32;
+ u.sentry.base = s32.base;
+ u.sentry.size = s32.size;
+ u.sentry.type = s32.type;
+ }
break;
- }
#endif
+ default:
+ return -ENOTTY;
+ }
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+ break;
+ case _IOC_NR(MTRRIOC_GET_ENTRY):
+ case _IOC_NR(MTRRIOC_GET_PAGE_ENTRY):
+ if (_IOC_DIR(cmd) != (_IOC_READ|_IOC_WRITE))
+ return -ENOTTY;
+ switch (ioctl_size) {
+ case sizeof(struct mtrr_gentry):
+ break;
+#ifdef CONFIG_COMPAT
+ case sizeof(struct mtrr_gentry32):
+ {
+ struct mtrr_gentry32 g32 = u.gentry32;
+ u.gentry.base = g32.base;
+ u.gentry.size = g32.size;
+ u.gentry.regnum = g32.regnum;
+ u.gentry.type = g32.type;
+ }
+ break;
+#endif
+ default:
+ return -ENOTTY;
+ }
+ break;
+ default:
+ return -ENOTTY;
}
- switch (cmd) {
+ switch (ioctl_nr) {
default:
return -ENOTTY;
- case MTRRIOC_ADD_ENTRY:
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
+ case _IOC_NR(MTRRIOC_ADD_ENTRY):
err =
- mtrr_file_add(sentry.base, sentry.size, sentry.type, 1,
+ mtrr_file_add(u.sentry.base, u.sentry.size, u.sentry.type, 1,
file, 0);
break;
- case MTRRIOC_SET_ENTRY:
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
- err = mtrr_add(sentry.base, sentry.size, sentry.type, 0);
+ case _IOC_NR(MTRRIOC_SET_ENTRY):
+ err = mtrr_add(u.sentry.base, u.sentry.size, u.sentry.type, 0);
break;
- case MTRRIOC_DEL_ENTRY:
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
- err = mtrr_file_del(sentry.base, sentry.size, file, 0);
+ case _IOC_NR(MTRRIOC_DEL_ENTRY):
+ err = mtrr_file_del(u.sentry.base, u.sentry.size, file, 0);
break;
- case MTRRIOC_KILL_ENTRY:
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
- err = mtrr_del(-1, sentry.base, sentry.size);
+ case _IOC_NR(MTRRIOC_KILL_ENTRY):
+ err = mtrr_del(-1, u.sentry.base, u.sentry.size);
break;
- case MTRRIOC_GET_ENTRY:
- if (gentry.regnum >= num_var_ranges)
+ case _IOC_NR(MTRRIOC_GET_ENTRY):
+ if (u.gentry.regnum >= num_var_ranges)
return -EINVAL;
- mtrr_if->get(gentry.regnum, &gentry.base, &gentry.size, &type);
+ mtrr_if->get(u.gentry.regnum, &u.gentry.base, &u.gentry.size, &type);
/* Hide entries that go above 4GB */
- if (gentry.base + gentry.size > 0x100000
- || gentry.size == 0x100000)
- gentry.base = gentry.size = gentry.type = 0;
+ if (u.gentry.base + u.gentry.size > 0x100000
+ || u.gentry.size == 0x100000)
+ u.gentry.base = u.gentry.size = u.gentry.type = 0;
else {
- gentry.base <<= PAGE_SHIFT;
- gentry.size <<= PAGE_SHIFT;
- gentry.type = type;
+ u.gentry.base <<= PAGE_SHIFT;
+ u.gentry.size <<= PAGE_SHIFT;
+ u.gentry.type = type;
}
break;
- case MTRRIOC_ADD_PAGE_ENTRY:
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
+ case _IOC_NR(MTRRIOC_ADD_PAGE_ENTRY):
err =
- mtrr_file_add(sentry.base, sentry.size, sentry.type, 1,
+ mtrr_file_add(u.sentry.base, u.sentry.size, u.sentry.type, 1,
file, 1);
break;
- case MTRRIOC_SET_PAGE_ENTRY:
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
- err = mtrr_add_page(sentry.base, sentry.size, sentry.type, 0);
+ case _IOC_NR(MTRRIOC_SET_PAGE_ENTRY):
+ err = mtrr_add_page(u.sentry.base, u.sentry.size, u.sentry.type, 0);
break;
- case MTRRIOC_DEL_PAGE_ENTRY:
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
- err = mtrr_file_del(sentry.base, sentry.size, file, 1);
+ case _IOC_NR(MTRRIOC_DEL_PAGE_ENTRY):
+ err = mtrr_file_del(u.sentry.base, u.sentry.size, file, 1);
break;
- case MTRRIOC_KILL_PAGE_ENTRY:
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
- err = mtrr_del_page(-1, sentry.base, sentry.size);
+ case _IOC_NR(MTRRIOC_KILL_PAGE_ENTRY):
+ err = mtrr_del_page(-1, u.sentry.base, u.sentry.size);
break;
- case MTRRIOC_GET_PAGE_ENTRY:
- if (gentry.regnum >= num_var_ranges)
+ case _IOC_NR(MTRRIOC_GET_PAGE_ENTRY):
+ if (u.gentry.regnum >= num_var_ranges)
return -EINVAL;
- mtrr_if->get(gentry.regnum, &gentry.base, &gentry.size, &type);
- gentry.type = type;
+ mtrr_if->get(u.gentry.regnum, &u.gentry.base, &u.gentry.size, &type);
+ u.gentry.type = type;
break;
}
if (err)
return err;
- switch(cmd) {
- case MTRRIOC_GET_ENTRY:
- case MTRRIOC_GET_PAGE_ENTRY:
- if (copy_to_user(arg, &gentry, sizeof gentry))
- err = -EFAULT;
- break;
+ if (ioctl_dir & _IOC_READ) {
+ switch (ioctl_size) {
#ifdef CONFIG_COMPAT
- case MTRRIOC32_GET_ENTRY:
- case MTRRIOC32_GET_PAGE_ENTRY: {
- struct mtrr_gentry32 __user *g32 = (struct mtrr_gentry32 __user *)__arg;
- err = put_user(gentry.base, &g32->base);
- err |= put_user(gentry.size, &g32->size);
- err |= put_user(gentry.regnum, &g32->regnum);
- err |= put_user(gentry.type, &g32->type);
+ case sizeof(struct mtrr_gentry32):
+ {
+ struct mtrr_gentry g64 = u.gentry;
+ u.gentry32.base = g64.base;
+ u.gentry32.size = g64.size;
+ u.gentry32.regnum = g64.regnum;
+ u.gentry32.type = g64.type;
+ }
break;
- }
#endif
+ default:
+ break;
+ }
+ if (copy_to_user(arg, &u, ioctl_size))
+ err = -EFAULT;
}
return err;