Skip to contentSkip to conversation

i8k: fix 64/32-bit compat mode ioctls

PATCH New Mark Lord 1 participant 1 message Dec 11, 2011 Char and misc drivers

lore
Share & tools
lore

Mark Lord

author
#0 lore raw

PATCH 1/1 i8k: fix 64/32-bit compat mode ioctls

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.

---

Patch is also attached to bypass email mangling
that may occur with the inline copy below.

drivers/char/i8k.c +41 -17
@@ -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.

drivers/char/i8k.c +41 -17

134 lines not on this page · raw message