From: Mark Lord <kernel@teksavvy.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: Linux 3.3-rc5
Date: Thu, 01 Mar 2012 22:48:09 -0500 [thread overview]
Message-ID: <4F5042F9.1050101@teksavvy.com> (raw)
In-Reply-To: <CA+55aFw8GLGtQ+rVi6eqbgyd+qKc3CszMkGv8L+2MC28K61O5Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3285 bytes --]
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:
[-- Attachment #2: 07_i8k_ioctl_fixes.patch --]
[-- Type: text/x-patch, Size: 3002 bytes --]
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:
next prev parent reply other threads:[~2012-03-02 3:48 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-25 20:34 Linux 3.3-rc5 Linus Torvalds
2012-02-27 15:23 ` Mark Lord
2012-02-28 18:30 ` Linus Torvalds
2012-02-28 19:29 ` Andreas Schwab
2012-02-28 19:39 ` Linus Torvalds
2012-03-01 14:47 ` Mark Lord
2012-03-01 15:26 ` [PATCH] i8k: fix ioctl handing (was Re: Linux 3.3-rc5) Mark Lord
2012-03-02 3:25 ` Linus Torvalds
2012-03-01 16:22 ` Linux 3.3-rc5 Linus Torvalds
2012-03-02 0:17 ` Mark Lord
2012-03-02 0:21 ` Mark Lord
2012-03-02 0:31 ` Linus Torvalds
2012-03-02 3:48 ` Mark Lord [this message]
2012-02-27 21:59 ` Michael S. Tsirkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F5042F9.1050101@teksavvy.com \
--to=kernel@teksavvy.com \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.