lkml.org 
[lkml]   [2012]   [Mar]   [1]   [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-03-01 07:31 PM, Linus Torvalds wrote:
> On Thu, Mar 1, 2012 at 4:21 PM, Mark Lord <kernel@teksavvy.com> wrote:
>>
>> Here's the promised strace from 64/32 mode:
>
> This is odd.
>
>> ioctl(0x3, 0xc0046986, 0xffc06dcc) = -1 (errno 22)
>
> When I do a "make drivers/char/i8k.s" (with my patch), I can see that
> constant in the resulting code. It's right there. It doesn't return
> EINVAL.
>
> Can you send me the patch you are using with the compat_ioctl thing added?

Yeah.. here's the version I was testing with just now,
which generated the errno above:

--- old/include/linux/i8k.h
+++ linux/include/linux/i8k.h
@@ -20,14 +20,24 @@
#define I8K_PROC "/proc/i8k"
#define I8K_PROC_FMT "1.0"

+/*
+ * For hysterical raisins we expose the wrong size to user space
+ * in the ioctl numbering. The actual real data size is "int".
+ */
+#ifdef __KERNEL__
+#define borked_i8k_arg int
+#else
+#define borked_i8k_arg size_t
+#endif
+
#define I8K_BIOS_VERSION _IOR ('i', 0x80, int) /* broken: meant 4 bytes */
#define I8K_MACHINE_ID _IOR ('i', 0x81, int) /* broken: meant 16 bytes */
-#define I8K_POWER_STATUS _IOR ('i', 0x82, size_t)
-#define I8K_FN_STATUS _IOR ('i', 0x83, size_t)
-#define I8K_GET_TEMP _IOR ('i', 0x84, size_t)
-#define I8K_GET_SPEED _IOWR('i', 0x85, size_t)
-#define I8K_GET_FAN _IOWR('i', 0x86, size_t)
-#define I8K_SET_FAN _IOWR('i', 0x87, size_t)
+#define I8K_POWER_STATUS _IOR ('i', 0x82, borked_i8k_arg)
+#define I8K_FN_STATUS _IOR ('i', 0x83, borked_i8k_arg)
+#define I8K_GET_TEMP _IOR ('i', 0x84, borked_i8k_arg)
+#define I8K_GET_SPEED _IOWR('i', 0x85, borked_i8k_arg)
+#define I8K_GET_FAN _IOWR('i', 0x86, borked_i8k_arg)
+#define I8K_SET_FAN _IOWR('i', 0x87, borked_i8k_arg)

#define I8K_FAN_LEFT 1
#define I8K_FAN_RIGHT 0
--- old/drivers/char/i8k.c 2012-03-01 09:44:03.400800231 -0500
+++ linux/drivers/char/i8k.c 2012-03-01 10:17:26.909946304 -0500
@@ -92,6 +92,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, unsigned long arg);
+
+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, arg);
+ mutex_unlock(&i8k_mutex);
+
+ return ret;
+}
+#endif
+
static const struct file_operations i8k_fops = {
.owner = THIS_MODULE,
.open = i8k_open_fs,
@@ -99,6 +116,9 @@
.llseek = seq_lseek,
.release = single_release,
.unlocked_ioctl = i8k_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = i8k_compat_ioctl,
+#endif
};

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

if (!argp)
return -EINVAL;

+ /* We had some bad 64-bit confusion */
+ if (((cmd >> _IOC_SIZESHIFT) & _IOC_SIZEMASK) == 8)
+ cmd -= 4 << _IOC_SIZESHIFT;
+
switch (cmd) {
case I8K_BIOS_VERSION:
- val = i8k_get_bios_version();
+ for (val = 0, i = 0; i < strlen(bios_version); i++)
+ val = (val << 8) | bios_version[i];
break;

case I8K_MACHINE_ID:Fix all kinds of badness in the i8k driver's ioctl handling.
This enables it to continue to work with existing 32-bit binaries
under both 32-bit and 64-bit kernels.

It also fixes the broken I8K_BIOS_VERSION call for all situations.

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

--- old/include/linux/i8k.h
+++ linux/include/linux/i8k.h
@@ -20,14 +20,24 @@
#define I8K_PROC "/proc/i8k"
#define I8K_PROC_FMT "1.0"

+/*
+ * For hysterical raisins we expose the wrong size to user space
+ * in the ioctl numbering. The actual real data size is "int".
+ */
+#ifdef __KERNEL__
+#define borked_i8k_arg int
+#else
+#define borked_i8k_arg size_t
+#endif
+
#define I8K_BIOS_VERSION _IOR ('i', 0x80, int) /* broken: meant 4 bytes */
#define I8K_MACHINE_ID _IOR ('i', 0x81, int) /* broken: meant 16 bytes */
-#define I8K_POWER_STATUS _IOR ('i', 0x82, size_t)
-#define I8K_FN_STATUS _IOR ('i', 0x83, size_t)
-#define I8K_GET_TEMP _IOR ('i', 0x84, size_t)
-#define I8K_GET_SPEED _IOWR('i', 0x85, size_t)
-#define I8K_GET_FAN _IOWR('i', 0x86, size_t)
-#define I8K_SET_FAN _IOWR('i', 0x87, size_t)
+#define I8K_POWER_STATUS _IOR ('i', 0x82, borked_i8k_arg)
+#define I8K_FN_STATUS _IOR ('i', 0x83, borked_i8k_arg)
+#define I8K_GET_TEMP _IOR ('i', 0x84, borked_i8k_arg)
+#define I8K_GET_SPEED _IOWR('i', 0x85, borked_i8k_arg)
+#define I8K_GET_FAN _IOWR('i', 0x86, borked_i8k_arg)
+#define I8K_SET_FAN _IOWR('i', 0x87, borked_i8k_arg)

#define I8K_FAN_LEFT 1
#define I8K_FAN_RIGHT 0
--- old/drivers/char/i8k.c 2012-03-01 09:44:03.400800231 -0500
+++ linux/drivers/char/i8k.c 2012-03-01 10:17:26.909946304 -0500
@@ -92,6 +92,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, unsigned long arg);
+
+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, arg);
+ mutex_unlock(&i8k_mutex);
+
+ return ret;
+}
+#endif
+
static const struct file_operations i8k_fops = {
.owner = THIS_MODULE,
.open = i8k_open_fs,
@@ -99,6 +116,9 @@
.llseek = seq_lseek,
.release = single_release,
.unlocked_ioctl = i8k_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = i8k_compat_ioctl,
+#endif
};

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

if (!argp)
return -EINVAL;

+ /* We had some bad 64-bit confusion */
+ if (((cmd >> _IOC_SIZESHIFT) & _IOC_SIZEMASK) == 8)
+ cmd -= 4 << _IOC_SIZESHIFT;
+
switch (cmd) {
case I8K_BIOS_VERSION:
- val = i8k_get_bios_version();
+ for (val = 0, i = 0; i < strlen(bios_version); i++)
+ val = (val << 8) | bios_version[i];
break;

case I8K_MACHINE_ID:
\
 
 \ /
  Last update: 2012-03-02 04:51    [W:0.148 / U:0.876 seconds]
©2003-2014 Jasper Spaans. Advertise on this site