lkml.org 
[lkml]   [2012]   [Feb]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patches in this message
    /
    Date
    From
    SubjectRe: Linux 3.3-rc5
    On 12-02-25 03:34 PM, Linus Torvalds wrote:
    ..
    > And while the FP state save problem is gone, if you have a 64-bit
    > capable CPU but are still running a 32-bit distro, it really would be
    > interesting to verify that a 64-bit kernel works for you without
    > problems. Because it always *should* have, but clearly that wasn't
    > always the case. It would be very interesting to hear from people who
    > have perhaps tried and failed before and perhaps didn't even bother to
    > report the failure? Maybe it's worth trying again?
    ..

    The i8k driver is still b0rked for COMPAT use in linux-3.2.xx,
    and I don't think my patch got picked up by anyone for 3.3 yet:

    (unmangled copy attached -- copy below is for viewing only)

    -----snip-----

    Fix the i8k (Dell Laptop) driver so that its misdefined ioctl's
    work with existing 32-bit userspace on a 64-bit kernel.

    Without this patch, 32-bit binaries get EINVAL from most ioctls,
    due to incompatible ioctl numbering and lack of a compat_ioctl() function.

    The problem is that the ioctls were originally defined using _IOR/_IOW
    macros, but the code is written assuming simpler _IO macros. Ugh.
    Too late to re-do all of that -- would break existing userspace.

    So just fix things up so they work again.

    This has been tested with existing pre-compiled binaries (i8kctl)
    on both a pure 32-bit system and a 64/32 system.

    Signed-off-by: Mark Lord <mlord@pobox.com>

    --- old/drivers/char/i8k.c 2011-11-28 17:47:43.000000000 -0500
    +++ linux/drivers/char/i8k.c 2011-12-10 17:42:58.463023349 -0500
    @@ -29,6 +29,7 @@
    #include <linux/mutex.h>
    #include <linux/hwmon.h>
    #include <linux/hwmon-sysfs.h>
    +#include <linux/compat.h>
    #include <asm/uaccess.h>
    #include <asm/io.h>

    @@ -92,6 +93,23 @@
    static int i8k_open_fs(struct inode *inode, struct file *file);
    static long i8k_ioctl(struct file *, unsigned int, unsigned long);

    +#ifdef CONFIG_COMPAT
    +static long
    +i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, int __user *argp);
    +
    +static long
    +i8k_compat_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
    +{
    + long ret;
    +
    + mutex_lock(&i8k_mutex);
    + ret = i8k_ioctl_unlocked(fp, cmd, compat_ptr(arg));
    + mutex_unlock(&i8k_mutex);
    +
    + return ret;
    +}
    +#endif
    +
    static const struct file_operations i8k_fops = {
    .owner = THIS_MODULE,
    .open = i8k_open_fs,
    @@ -99,6 +117,9 @@
    .llseek = seq_lseek,
    .release = single_release,
    .unlocked_ioctl = i8k_ioctl,
    +#ifdef CONFIG_COMPAT
    + .compat_ioctl = i8k_compat_ioctl,
    +#endif
    };

    struct smm_regs {
    @@ -315,54 +336,56 @@
    return regs.eax == 1145651527 && regs.edx == 1145392204 ? 0 : -1;
    }

    -static int
    -i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg)
    +static long
    +i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, int __user *argp)
    {
    int val = 0;
    int speed;
    unsigned char buff[16];
    - int __user *argp = (int __user *)arg;

    if (!argp)
    return -EINVAL;

    - switch (cmd) {
    - case I8K_BIOS_VERSION:
    - val = i8k_get_bios_version();
    + switch (_IOC_NR(cmd)) {
    + case (_IOC_NR(I8K_BIOS_VERSION)):
    + {
    + int i;
    + for (val = 0, i = 0; i < strlen(bios_version); i++)
    + val = (val << 8) | bios_version[i];
    break;
    -
    - case I8K_MACHINE_ID:
    + }
    + case (_IOC_NR(I8K_MACHINE_ID)):
    memset(buff, 0, 16);
    strlcpy(buff, i8k_get_dmi_data(DMI_PRODUCT_SERIAL), sizeof(buff));
    break;

    - case I8K_FN_STATUS:
    + case (_IOC_NR(I8K_FN_STATUS)):
    val = i8k_get_fn_status();
    break;

    - case I8K_POWER_STATUS:
    + case (_IOC_NR(I8K_POWER_STATUS)):
    val = i8k_get_power_status();
    break;

    - case I8K_GET_TEMP:
    + case (_IOC_NR(I8K_GET_TEMP)):
    val = i8k_get_temp(0);
    break;

    - case I8K_GET_SPEED:
    + case (_IOC_NR(I8K_GET_SPEED)):
    if (copy_from_user(&val, argp, sizeof(int)))
    return -EFAULT;

    val = i8k_get_fan_speed(val);
    break;

    - case I8K_GET_FAN:
    + case (_IOC_NR(I8K_GET_FAN)):
    if (copy_from_user(&val, argp, sizeof(int)))
    return -EFAULT;

    val = i8k_get_fan_status(val);
    break;

    - case I8K_SET_FAN:
    + case (_IOC_NR(I8K_SET_FAN)):
    if (restricted && !capable(CAP_SYS_ADMIN))
    return -EPERM;

    @@ -383,12 +406,12 @@
    return val;

    switch (cmd) {
    - case I8K_BIOS_VERSION:
    + case (_IOC_NR(I8K_BIOS_VERSION)):
    if (copy_to_user(argp, &val, 4))
    return -EFAULT;

    break;
    - case I8K_MACHINE_ID:
    + case (_IOC_NR(I8K_MACHINE_ID)):
    if (copy_to_user(argp, buff, 16))
    return -EFAULT;

    @@ -406,9 +429,10 @@
    static long i8k_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
    {
    long ret;
    + int __user *argp = (int __user *)arg;

    mutex_lock(&i8k_mutex);
    - ret = i8k_ioctl_unlocked(fp, cmd, arg);
    + ret = i8k_ioctl_unlocked(fp, cmd, argp);
    mutex_unlock(&i8k_mutex);

    return ret;Fix the i8k (Dell Laptop) driver so that its misdefined ioctl's
    work with existing 32-bit userspace on a 64-bit kernel.

    Without this patch, 32-bit binaries get EINVAL from most ioctls,
    due to incompatible ioctl numbering and lack of a compat_ioctl() function.

    The problem is that the ioctls were originally defined using _IOR/_IOW
    macros, but the code is written assuming simpler _IO macros. Ugh.
    Too late to re-do all of that -- would break existing userspace.

    So just fix things up so they work again.

    This has been tested with existing pre-compiled binaries (i8kctl)
    on both a pure 32-bit system and a 64/32 system.

    Signed-off-by: Mark Lord <mlord@pobox.com>

    --- old/drivers/char/i8k.c 2011-11-28 17:47:43.000000000 -0500
    +++ linux/drivers/char/i8k.c 2011-12-10 17:42:58.463023349 -0500
    @@ -29,6 +29,7 @@
    #include <linux/mutex.h>
    #include <linux/hwmon.h>
    #include <linux/hwmon-sysfs.h>
    +#include <linux/compat.h>
    #include <asm/uaccess.h>
    #include <asm/io.h>

    @@ -92,6 +93,23 @@
    static int i8k_open_fs(struct inode *inode, struct file *file);
    static long i8k_ioctl(struct file *, unsigned int, unsigned long);

    +#ifdef CONFIG_COMPAT
    +static long
    +i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, int __user *argp);
    +
    +static long
    +i8k_compat_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
    +{
    + long ret;
    +
    + mutex_lock(&i8k_mutex);
    + ret = i8k_ioctl_unlocked(fp, cmd, compat_ptr(arg));
    + mutex_unlock(&i8k_mutex);
    +
    + return ret;
    +}
    +#endif
    +
    static const struct file_operations i8k_fops = {
    .owner = THIS_MODULE,
    .open = i8k_open_fs,
    @@ -99,6 +117,9 @@
    .llseek = seq_lseek,
    .release = single_release,
    .unlocked_ioctl = i8k_ioctl,
    +#ifdef CONFIG_COMPAT
    + .compat_ioctl = i8k_compat_ioctl,
    +#endif
    };

    struct smm_regs {
    @@ -315,54 +336,56 @@
    return regs.eax == 1145651527 && regs.edx == 1145392204 ? 0 : -1;
    }

    -static int
    -i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg)
    +static long
    +i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, int __user *argp)
    {
    int val = 0;
    int speed;
    unsigned char buff[16];
    - int __user *argp = (int __user *)arg;

    if (!argp)
    return -EINVAL;

    - switch (cmd) {
    - case I8K_BIOS_VERSION:
    - val = i8k_get_bios_version();
    + switch (_IOC_NR(cmd)) {
    + case (_IOC_NR(I8K_BIOS_VERSION)):
    + {
    + int i;
    + for (val = 0, i = 0; i < strlen(bios_version); i++)
    + val = (val << 8) | bios_version[i];
    break;
    -
    - case I8K_MACHINE_ID:
    + }
    + case (_IOC_NR(I8K_MACHINE_ID)):
    memset(buff, 0, 16);
    strlcpy(buff, i8k_get_dmi_data(DMI_PRODUCT_SERIAL), sizeof(buff));
    break;

    - case I8K_FN_STATUS:
    + case (_IOC_NR(I8K_FN_STATUS)):
    val = i8k_get_fn_status();
    break;

    - case I8K_POWER_STATUS:
    + case (_IOC_NR(I8K_POWER_STATUS)):
    val = i8k_get_power_status();
    break;

    - case I8K_GET_TEMP:
    + case (_IOC_NR(I8K_GET_TEMP)):
    val = i8k_get_temp(0);
    break;

    - case I8K_GET_SPEED:
    + case (_IOC_NR(I8K_GET_SPEED)):
    if (copy_from_user(&val, argp, sizeof(int)))
    return -EFAULT;

    val = i8k_get_fan_speed(val);
    break;

    - case I8K_GET_FAN:
    + case (_IOC_NR(I8K_GET_FAN)):
    if (copy_from_user(&val, argp, sizeof(int)))
    return -EFAULT;

    val = i8k_get_fan_status(val);
    break;

    - case I8K_SET_FAN:
    + case (_IOC_NR(I8K_SET_FAN)):
    if (restricted && !capable(CAP_SYS_ADMIN))
    return -EPERM;

    @@ -383,12 +406,12 @@
    return val;

    switch (cmd) {
    - case I8K_BIOS_VERSION:
    + case (_IOC_NR(I8K_BIOS_VERSION)):
    if (copy_to_user(argp, &val, 4))
    return -EFAULT;

    break;
    - case I8K_MACHINE_ID:
    + case (_IOC_NR(I8K_MACHINE_ID)):
    if (copy_to_user(argp, buff, 16))
    return -EFAULT;

    @@ -406,9 +429,10 @@
    static long i8k_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
    {
    long ret;
    + int __user *argp = (int __user *)arg;

    mutex_lock(&i8k_mutex);
    - ret = i8k_ioctl_unlocked(fp, cmd, arg);
    + ret = i8k_ioctl_unlocked(fp, cmd, argp);
    mutex_unlock(&i8k_mutex);

    return ret;
    \
     
     \ /
      Last update: 2012-02-27 16:25    [W:0.085 / U:29.628 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site