Linux 3.3-rc5

Linux 3.3-rc5

[Linus Torvalds]

Hey, no delays this week.

And nothing really odd going on either. Maybe things are finally calming down.

Sure, I'd have liked it to be even calmer, and there's is movement in
various areas: btrfs updates along with scsi and media driver updates.
And various noise elsewhere. But on the whole it's been pretty boring,
which is just how I like it.

A couple of the fixes were directly linked to my -rc4 announcement,
where I told people they could use a 64-bit kernel to avoid the FP
state save/restore problem we used to have. That brought up some
fairly corner-case compatibility issues that could make that
impractical. So hopefully it's *true* this time around.

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?

                          Linus

---
Al Viro (2):
      vfs: fix compat_sys_stat() handling of overflows in st_nlink
      ocfs2: deal with wraparounds of i_nlink in ocfs2_rename()

Alan Stern (2):
      [SCSI] scsi_pm: Fix bug in the SCSI power management handler
      usb-storage: fix freezing of the scanning thread

Alex Deucher (1):
      drm/radeon/kms/atom: dpms bios scratch reg updates

Alexander Stein (2):
      m68k: Add shared bit to Coldfire kernel page entries
      m68k: Do not set global share for non-kernel shared pages

Alexey Khoroshilov (1):
      [SCSI] mpt2sas: Fix mismatch in
mpt2sas_base_hard_reset_handler() mutex lock-unlock

Andrew Lunn (1):
      USB: Serial: ti_usb_3410_5052: Add Abbot Diabetes Care cable id

Andrew Vasquez (4):
      [SCSI] qla2xxx: Add an "is reset active" helper.
      [SCSI] qla2xxx: Clear options-flags while issuing stop-firmware
mbx command.
      [SCSI] qla2xxx: Remove errant clearing of MBX_INTERRUPT flag
during CT-IOCB processing.
      [SCSI] qla2xxx: Correct out of bounds read of ISP2200 mailbox registers.

Anton Altaparmakov (1):
      Restore direct_io / truncate locking API

Anton Vorontsov (1):
      mm: memcg: Correct unregistring of events attached to the same eventfd

Arne Jansen (1):
      btrfs: don't check DUP chunks twice

Arun Easi (1):
      [SCSI] qla2xxx: Propagate up abort failures.

Ben Hutchings (1):
      builddeb: Don't create files in /tmp with predictable names

Benjamin Herrenschmidt (2):
      cpuidle: Default y on powerpc pSeries
      powerpc: Fix various issues with return to userspace

Bruno Thomsen (1):
      USB: Added Kamstrup VID/PIDs to cp210x serial driver.

Catalin Marinas (1):
      ARM: 7323/1: Do not allow ARM_LPAE on pre-ARMv7 architectures

Chad Dupuis (2):
      [SCSI] qla2xxx: Add check for null fcport references in
qla2xxx_queuecommand.
      [SCSI] qla2xxx: Update version number to 8.03.07.13-k.

Chris D Schimp (2):
      hwmon: (max6639) Fix FAN_FROM_REG calculation
      hwmon: (max6639) Fix PPR register initialization to set both channels

Chris Mason (6):
      Btrfs: don't reserve data with extents locked in btrfs_fallocate
      Btrfs: improve error handling for btrfs_insert_dir_item callers
      Btrfs: make sure we update latest_bdev
      Btrfs: add extra sanity checks on the path names in btrfs_mksubvol
      Btrfs: clear the extent uptodate bits during parent transid failures
      Btrfs: fix compiler warnings on 32 bit systems

Danny Kukawka (1):
      arch/arm/mach-shmobile/board-ag5evm.c: included linux/dma-mapping.h twice

Dave Jiang (1):
      [SCSI] isci: Fix NULL ptr dereference when no firmware is being loaded

David Howells (2):
      NOMMU: Lock i_mmap_mutex for access to the VMA prio list
      NOMMU: Don't need to clear vm_mm when deleting a VMA

David Sterba (2):
      btrfs: fix structs where bitfields and spinlock/atomic share 8B word
      btrfs: silence warning in raid array setup

Dimitri Sivanich (1):
      vfs: fix panic in __d_lookup() with high dentry hashtable counts

Dmitry Kasatkin (1):
      digsig: changed type of the timestamp

Doug Ledford (1):
      mlx4_core: Exported functions can't be static

Elric Fu (1):
      USB: Set hub depth after USB3 hub reset

Eric Paris (1):
      ARM/audit: include audit header and fix audit arch

Eugeni Dodonov (4):
      drm/i915: gen7: implement rczunit workaround
      drm/i915: gen7: Implement an L3 caching workaround.
      drm/i915: gen7: work around a system hang on IVB
      drm/i915: do not enable RC6p on Sandy Bridge

Florian Albrechtskirchinger (1):
      btrfs: honor umask when creating subvol root

Giridhar Malavali (2):
      [SCSI] qla2xxx: Complete mailbox command timedout to avoid
initialization failures during next reset cycle.
      [SCSI] qla2xxx: Proper detection of firmware abort error code for ISP82xx.

Greg Dietsche (1):
      coccicheck: change handling of C={1,2} when M= is set

Greg Ungerer (1):
      m68knommu: fix syscall tracing stuck process

Guennadi Liakhovetski (4):
      ARM: mach-shmobile: simplify MMCIF DMA configuration
      sh: sh7757lcr: update to the new MMCIF DMA configuration
      ARM: mach-shmobile: add GPIO-to-IRQ translation to sh7372
      sh: se7724: fix compile breakage

Guenter Roeck (2):
      hwmon: (ads1015) Fix file leak in probe function
      hwmon: (max34440) Fix resetting temperature history

Heiko Carstens (1):
      [S390] idle: avoid RCU usage in extended quiescent state

Hitoshi Mitake (1):
      asm-generic: architecture independent readq/writeq for 32bit environment

Huajun Li (1):
      [SCSI] scsi_scan: Fix 'Poison overwritten' warning caused by
using freed 'shost'

Ian Kent (1):
      autofs: work around unhappy compat problem on x86-64

Inki Dae (4):
      drm/exynos: added possible_clones setup function.
      drm/exynos: fixed page flip issue.
      drm/exynos: removed exynos_drm_fbdev_recreate function.
      drm/exynos: added postclose to release resource.

James Morris (1):
      maintainers: update my email address

Jan Kara (2):
      vfs: Provide function to get superblock and wait for it to thaw
      quota: Fix deadlock with suspend and quotas

Jan Schmidt (1):
      Btrfs: avoid positive number with ERR_PTR

Janne Grunau (1):
      [media] hdpvr: fix race conditon during start of streaming

Jarod Wilson (1):
      [media] imon: don't wedge hardware after early callbacks

Javi Merino (2):
      ARM: 7164/3: PL330: Fix the size of the dst_cache_ctrl field
      ARM: 7326/2: PL330: fix null pointer dereference in pl330_chan_ctrl()

Jeff Liu (1):
      Btrfs: return the internal error unchanged if
btrfs_get_extent_fiemap() call failed for SEEK_DATA/SEEK_HOLE inquiry

Jeff Mahoney (1):
      btrfs: delalloc for page dirtied out-of-band in fixup worker

Jerome Glisse (1):
      drm/radeon/kms: properly set accel working flag and bailout when false

Jesper Juhl (1):
      XFS: xfs_trans_add_item() - don't assign in ASSERT() when
compare is intended

Joonyoung Shim (2):
      drm/exynos: changed priority of mixer layers.
      drm/exynos: removed pageflip_event_list init code when closed.

Keith Mannthey (1):
      btrfs: Sector Size check during Mount

Kenneth Graunke (1):
      drm/i915: gen7: Disable the RHWO optimization as it can cause GPU hangs.

Kleber Sacilotto de Souza (1):
      [SCSI] ipr: fix eeh recovery for 64-bit adapters

Kuninori Morimoto (6):
      sh: clock-sh7724: fixup sh_fsi clock settings
      ARM: mach-shmobile: mackerel: use renesas_usbhs instead of r8a66597_hcd
      ARM: mach-shmobile: mackerel: add ak4642 amixer settings on comment
      ARM: mach-shmobile: clock-sh73a0: add DSIxPHY clock support
      sh: clkfwk: bugfix: use clk_reparent() for div6 clocks
      ASoC: ak4642: fixup HeadPhone L/R dapm settings

Kyle McMartin (1):
      MAINTAINERS: drop me from PA-RISC maintenance

Laurent Pinchart (1):
      [media] omap3isp: Fix crash caused by subdevs now having a
pointer to devnodes

Linus Torvalds (6):
      i387: fix up some fpu_counter confusion
      i387: use 'restore_fpu_checking()' directly in task switching code
      i387: support lazy restore of FPU state
      i387: export 'fpu_owner_task' per-cpu variable
      sys_poll: fix incorrect type for 'timeout' parameter
      Linux 3.3-rc5

Liu Bo (6):
      Btrfs: fix trim 0 bytes after a device delete
      Btrfs: skip states when they does not contain bits to clear
      Btrfs: kick out redundant stuff in convert_extent_bit
      Btrfs: fix a bug on overcommit stuff
      Btrfs: be less strict on finding next node in clear_extent_bit
      Btrfs: increase the global block reserve estimates

Lucas De Marchi (1):
      kbuild: do not check for ancient modutils tools

Magnus Damm (5):
      ARM: mach-shmobile: sh73a0 PINT IRQ base fix
      ARM: mach-shmobile: sh73a0 IRQ sparse alloc fix
      ARM: mach-shmobile: IRQ driven GPIO key support for Kota2
      ARM: mach-shmobile: sh73a0 PSTR 32-bit access fix
      ARM: mach-shmobile: r8a7779 PFC IPSR4 fix

Mark Brown (1):
      ASoC: wm8962: Fix sidetone enumeration texts

Mark Hills (1):
      ALSA: snd-usb-caiaq: Fix the return of XRUN

Martin Schwidefsky (3):
      [S390] incorrect PageTables counter for kvm page tables
      [S390] 3215 deadlock with tty_wakeup
      [S390] correct ktime to tod clock comparator conversion

Masanari Iida (2):
      drm/exynos: Fix typo in exynos_mixer.c
      sh: Fix typo in pci-sh7780.c

Miao Xie (1):
      Btrfs: fix deadlock on page lock when doing auto-defragment

Michael Christie (1):
      [SCSI] qla2xxx: Remove check for null fcport from host reset handler.

Michael Ellerman (1):
      powerpc: Fix program check handling when lockdep is enabled

Michel Dänzer (1):
      drm/radeon: Only create additional ring debugfs files on Cayman or newer.

Miklos Szeredi (1):
      vfs: fix d_inode_lookup() dentry ref leak

Mitsuo Hayasaka (2):
      xfs: change available ranges of softlimit and hardlimit in quota check
      xfs: make inode quota check more general

Moger, Babu (1):
      [SCSI] scsi_dh_rdac: Fix for unbalanced reference count

Myron Stowe (1):
      ARM/PCI: Remove ARM's duplicate definition of 'pcibios_max_latency'

Nikolaus Schulz (1):
      hwmon: (f75375s) Fix register write order when setting fans to full speed

Oleg Nesterov (2):
      epoll: introduce POLLFREE to flush ->signalfd_wqh before kfree()
      epoll: ep_unregister_pollwait() can use the freed pwq->whead

Olof Johansson (1):
      ARM: 7327/1: need to include asm/system.h in asm/processor.h

Paul Gortmaker (1):
      arm: fix compile failure in mach-shmobile/board-ag5evm.c

Paul Mundt (2):
      sh: Defer to asm-generic/device.h.
      video: pvr2fb: Fix up spurious section mismatch warnings.

Phil Edworthy (1):
      sh: Fix sh2a build error for CONFIG_CACHE_WRITETHROUGH

Rabin Vincent (1):
      ARM: 7325/1: fix v7 boot with lockdep enabled

Randy Dunlap (1):
      [media] wl128x: fix build errors when GPIOLIB is not enabled

Russell King (1):
      ARM: OMAP: fix voltage domain build errors with PM_OPP disabled

Rusty Russell (2):
      powerpc: Remove references to cpu_*_map
      arch/sh: remove references to cpu_*_map.

Santosh Shilimkar (1):
      ARM: 7336/1: smp_twd: Don't register CPUFREQ notifiers if local
timers are not initialised

Sarah Sharp (5):
      USB: Remove duplicate USB 3.0 hub feature #defines.
      xhci: Fix oops caused by more USB2 ports than USB3 ports.
      USB: Don't fail USB3 probe on missing legacy PCI IRQ.
      USB: Fix handoff when BIOS disables host PCI device.
      xhci: Fix encoding for HS bulk/control NAK rate.

Shimoda, Yoshihiro (4):
      sh: fix the sh_mmcif_plat_data in board-sh7757lcr
      sh: modify resource for SPI0 in setup-sh7757
      sh: add platform_device for SPI1 in setup-sh7757
      sh: modify a resource of sh_eth_giga1_resources in board-sh7757lcr

Shyam Sundar (1):
      [SCSI] qla2xxx: Remove resetting memory during device
initialization for ISP82xx.

Stephen Warren (1):
      Kbuild: Use dtc's -d (dependency) option

Steven Rostedt (1):
      autofs4 - fix lockdep splat in autofs

Takashi Iwai (2):
      ALSA: hda/realtek - Fix overflow of vol/sw check bitmap
      ALSA: hda/realtek - Fix surround output regression on Acer Aspire 5935

Taylor Ralph (1):
      [media] hdpvr: update picture controls to support firmware versions > 0.15

Tony Lindgren (1):
      ARM: 7324/1: modpost: Fix section warnings for ARM for many compilers

Trond Myklebust (3):
      NFSv4: Fix an Oops in the NFSv4 getacl code
      NFSv4: Ensure we throw out bad delegation stateids on NFS4ERR_BAD_STATEID
      NFSv4.1: Fix a NFSv4.1 session initialisation regression

Tsutomu Itoh (3):
      Btrfs: fix memory leak in load_free_space_cache()
      Btrfs: fix return value check of extent_io_ops
      Btrfs: check return value of lookup_extent_mapping() correctly

Weston Andros Adamson (1):
      NFSv4: fix server_scope memory leak

Xi Wang (1):
      ALSA: usb-audio: avoid integer overflow in create_fixed_stream_quirk()

li.rui27@zte.com.cn (1):
      USB: option: cleanup zte 3g-dongle's pid in option.c

Re: Linux 3.3-rc5

[Mark Lord]

[-- Attachment #1: Type: text/plain, Size: 4669 bytes --]

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;

[-- Attachment #2: 07_i8k_64bit_fixes.patch --]
[-- Type: text/x-patch, Size: 3959 bytes --]

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;

Re: Linux 3.3-rc5

[Michael S. Tsirkin]

On Sat, Feb 25, 2012 at 12:34:20PM -0800, Linus Torvalds wrote:
> Hey, no delays this week.
> 
> And nothing really odd going on either. Maybe things are finally calming down.
> 
> Sure, I'd have liked it to be even calmer, and there's is movement in
> various areas: btrfs updates along with scsi and media driver updates.
> And various noise elsewhere. But on the whole it's been pretty boring,
> which is just how I like it.
> 
> A couple of the fixes were directly linked to my -rc4 announcement,
> where I told people they could use a 64-bit kernel to avoid the FP
> state save/restore problem we used to have. That brought up some
> fairly corner-case compatibility issues that could make that
> impractical. So hopefully it's *true* this time around.
> 
> 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?
> 
>                           Linus

Yes, I just tried.

IIRC, I used to run 64 bit upstream kernels with 32 bit userspace
under Fedora 14 (because I can) this stopped booting when I switched
to Fedora 15, I think the upstream kernel was 2.6.39 at that time,
so I stopped trying.

With 3.3.0-rc5, a 64 bit kernel works fine for me, again.
Yay!

As a side note, it's a pity that gcc in 32 bit Fedora
does not support -m 64 to support building 64 bit kernels.

-- 
MST

Re: Linux 3.3-rc5

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 2251 bytes --]

On Mon, Feb 27, 2012 at 7:23 AM, Mark Lord <kernel@teksavvy.com> wrote:
>
> 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:

I really don't like your patch.

It basically compares the ioctl argument by dropping almost all bits.
That looks completely wrong. If I read the patch correctly, it will
now match ioctl's that have *nothing* to do with the i8k ioctls, that
just happen to have a few bits in common.

I also think that your explanation is wrong. The problem is not the
use of _IOR() at all, the problem is that the data structure given
*to* the _IOR() macro is complete grabage. For example:

  #define I8K_GET_SPEED           _IOWR('i', 0x85, size_t)

that "size_t" there is just crap. It's wrong. The ioctl doesn't write
a size_t (which is different sizes on 32-bit and 64-bit), it actually
writes an "int" (which happens to be the same size in both compat and
non-compat).

So the 64-bit code - totally incorrectly - has been compiled with an
ioctl number that implies a 64-bit argument.

EVERY SINGLE IOCTL that is defined for the i8k driver is crap. There
are two that were "int", but they are marked as "broken" because the
data structure *they* write isn't actually "int". But those two are
the ones that just happen to work in both 32-bit and 64-bit mode,
because at least the size of the (incorrect) data structure is the
same.

The ones that have "size_t" aren't marked broken, but those are the
*really* broken ones due to the wrong data structure choice that has a
size that now is different in 32-bit and 64-bit mode.

Quite frankly, I think the right solution is to fix the kernel
interface to the right type (int) that is the same. But because we
don't want to change the user interface, let's make the kernel
*accept* the 8-byte entity and just change it into a 4-byte size, and
leave the user-space visible ioctl numbers the same broken crap - it's
not like the other ioctl numbers had the right size *either*...

IOW, have something like the attached in the ioctl handler (and then
we need to also add the compat handler, as in your patch).

Does this (with your compat_ioctl wrapper addition) also work for you?

                        Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1907 bytes --]

 drivers/char/i8k.c  |    4 ++++
 include/linux/i8k.h |   22 ++++++++++++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
index 40cc0cf2ded6..e4b1613543ed 100644
--- a/drivers/char/i8k.c
+++ b/drivers/char/i8k.c
@@ -326,6 +326,10 @@ i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg)
 	if (!argp)
 		return -EINVAL;
 
+	/* We had some bad 64-bit confusion */
+	if ((arg >> _IOC_SIZESHIFT) & _IOC_SIZEMASK) == 8)
+		arg -= 4 << _IOC_SIZESHIFT;
+
 	switch (cmd) {
 	case I8K_BIOS_VERSION:
 		val = i8k_get_bios_version();
diff --git a/include/linux/i8k.h b/include/linux/i8k.h
index 1c45ba505115..650f4015c89c 100644
--- a/include/linux/i8k.h
+++ b/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

Re: Linux 3.3-rc5

[Andreas Schwab]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> I also think that your explanation is wrong. The problem is not the
> use of _IOR() at all, the problem is that the data structure given
> *to* the _IOR() macro is complete grabage. For example:
>
>   #define I8K_GET_SPEED           _IOWR('i', 0x85, size_t)

Originally the garbage looked like this:

#define I8K_BIOS_VERSION       _IOR ('i', 0x80, 4)
#define I8K_MACHINE_ID         _IOR ('i', 0x81, 16)
#define I8K_POWER_STATUS       _IOR ('i', 0x82, sizeof(int))
#define I8K_FN_STATUS          _IOR ('i', 0x83, sizeof(int))
#define I8K_GET_TEMP           _IOR ('i', 0x84, sizeof(int))
#define I8K_GET_SPEED          _IOWR('i', 0x85, sizeof(int))
#define I8K_GET_FAN            _IOWR('i', 0x86, sizeof(int))
#define I8K_SET_FAN            _IOWR('i', 0x87, sizeof(int)*2)

Later they were modified by "[PATCH] use size_t for the broken ioctl
numbers" and "Fix more ioctl _IOR/_IOW misusage." (see tglx's history
tree).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Linux 3.3-rc5

[Linus Torvalds]

On Tue, Feb 28, 2012 at 11:29 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> Originally the garbage looked like this:
>
> #define I8K_BIOS_VERSION       _IOR ('i', 0x80, 4)
> #define I8K_MACHINE_ID         _IOR ('i', 0x81, 16)
> #define I8K_POWER_STATUS       _IOR ('i', 0x82, sizeof(int))
> #define I8K_FN_STATUS          _IOR ('i', 0x83, sizeof(int))
> #define I8K_GET_TEMP           _IOR ('i', 0x84, sizeof(int))
> #define I8K_GET_SPEED          _IOWR('i', 0x85, sizeof(int))
> #define I8K_GET_FAN            _IOWR('i', 0x86, sizeof(int))
> #define I8K_SET_FAN            _IOWR('i', 0x87, sizeof(int)*2)
>
> Later they were modified by "[PATCH] use size_t for the broken ioctl
> numbers" and "Fix more ioctl _IOR/_IOW misusage." (see tglx's history
> tree).

Ahh, yes. That wasn't the only driver that did the "sizeof" by hand,
and as a result got "size_t" when the (nested) *real* sizeof then
happened.

That explains the history behind the garbage.

ioctl numbering have always been pretty painful. Oh, how I wish they
had been ASCII strings rather than random binary blobs. But that's the
way things are (and we certainly screwed up a few /proc files too, so
it's not like using text fixes all potential for screwing up)

                 Linus

Re: Linux 3.3-rc5

[Mark Lord]

On 12-02-28 01:30 PM, Linus Torvalds wrote:
> On Mon, Feb 27, 2012 at 7:23 AM, Mark Lord <kernel@teksavvy.com> wrote:
>>
>> 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:
>
> I really don't like your patch.
..

Heh, I think we agree on that point.  I hate it.  :)

> Quite frankly, I think the right solution is to fix the kernel
> interface to the right type (int) that is the same. But because we
> don't want to change the user interface, let's make the kernel
> *accept* the 8-byte entity and just change it into a 4-byte size, and
> leave the user-space visible ioctl numbers the same broken crap - it's
> not like the other ioctl numbers had the right size *either*...
>
> IOW, have something like the attached in the ioctl handler (and then
> we need to also add the compat handler, as in your patch).
>
> Does this (with your compat_ioctl wrapper addition) also work for you?


It's close (after adding a missing left-paren), but not 100% working yet.
In particular, this command fails to get valid data:  i8kctl bios

    ioctl(3, I8K_BIOS_VERSION, 0xbfc543c8)  = -1 EINVAL (Invalid argument)

That's the original issue I first noticed which prompted me to try
and get things to roughly behave again.

I think all (or at least most) of the other ioctls appear to be working though.

[PATCH] i8k: fix ioctl handing (was Re: Linux 3.3-rc5)

[Mark Lord]

[-- Attachment #1: Type: text/plain, Size: 4218 bytes --]

On 12-03-01 09:47 AM, Mark Lord wrote:
> On 12-02-28 01:30 PM, Linus Torvalds wrote:
>> On Mon, Feb 27, 2012 at 7:23 AM, Mark Lord <kernel@teksavvy.com> wrote:
..
>> Does this (with your compat_ioctl wrapper addition) also work for you?
>
>
> It's close (after adding a missing left-paren), but not 100% working yet.
> In particular, this command fails to get valid data:  i8kctl bios
>
>     ioctl(3, I8K_BIOS_VERSION, 0xbfc543c8)  = -1 EINVAL (Invalid argument)

Oh.. that's due to just plain broken code in i8k.c.
My original patch had a fix for that too:

-       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;

So, with your patch + bits of mine, we may have it all working.
Here's a single combined patch for further discussion.

I'm still not 100% happy with the arg/argp passing to i8ki_unlocked_ioctl() though.
Shouldn't there be a compat_ptr() macro in there someplace?

----------------snip-----------------

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 (((arg >> _IOC_SIZESHIFT) & _IOC_SIZEMASK) == 8)
+		arg -= 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 (((arg >> _IOC_SIZESHIFT) & _IOC_SIZEMASK) == 8)
+		arg -= 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:

Re: Linux 3.3-rc5

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 970 bytes --]

On Thu, Mar 1, 2012 at 6:47 AM, Mark Lord <kernel@teksavvy.com> wrote:
>
> It's close (after adding a missing left-paren), but not 100% working yet.
> In particular, this command fails to get valid data:  i8kctl bios
>
>    ioctl(3, I8K_BIOS_VERSION, 0xbfc543c8)  = -1 EINVAL (Invalid argument)

Hmm. That makes no sense.

I8K_BIOS_VERSION is 0x80046980 - and it already matches on both x86-64
and x86-32. I just checked by compiling to asm. And the size is '4',
so it wouldn't trigger any changes by the sizemask thing either.

Ugh. But my patch was crap. It fixed up "arg", but it *should* have
fixed up "cmd".

Stupid.

So try *this* one instead. I also fixed up the missing parenthesis,
but didn't add the actual .compat function etc.

Does *this* work? And if not, can you do an strace, but use the "-e
raw=ioctl" option to make it show the ioctl numbers as raw hex to
verify..

                     Linus

                        Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1908 bytes --]

 drivers/char/i8k.c  |    4 ++++
 include/linux/i8k.h |   22 ++++++++++++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
index 40cc0cf2ded6..2baa921afd84 100644
--- a/drivers/char/i8k.c
+++ b/drivers/char/i8k.c
@@ -326,6 +326,10 @@ i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long 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();
diff --git a/include/linux/i8k.h b/include/linux/i8k.h
index 1c45ba505115..650f4015c89c 100644
--- a/include/linux/i8k.h
+++ b/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

Re: Linux 3.3-rc5

[Mark Lord]

On 12-03-01 11:22 AM, Linus Torvalds wrote:
> On Thu, Mar 1, 2012 at 6:47 AM, Mark Lord <kernel@teksavvy.com> wrote:
>>
>> It's close (after adding a missing left-paren), but not 100% working yet.
>> In particular, this command fails to get valid data:  i8kctl bios
>>
>>    ioctl(3, I8K_BIOS_VERSION, 0xbfc543c8)  = -1 EINVAL (Invalid argument)
>
> Hmm. That makes no sense.

No, that was my misinterpretation.
The BIOS_VERSION call is broken for different reasons,
and my original all-inclusive patch has a fix for it too.  :)

> Does *this* work? And if not, can you do an strace, but use the "-e
> raw=ioctl" option to make it show the ioctl numbers as raw hex to
> verify..

Here's a go, 64-bit kernel (3.2.8), 32-bit userspace:

Dell laptop SMM driver v1.14 21/02/2005 Massimo Dal Zotto (dz@debian.org)
ioctl32(i8kctl:1474): Unknown cmd fd(3) cmd(80046980){t:'i';sz:4} arg(ff8b1fd8)
on /proc/i8k
ioctl32(i8kctl:1475): Unknown cmd fd(3) cmd(c0046986){t:'i';sz:4} arg(ff90ed5c)
on /proc/i8k
ioctl32(i8kctl:1475): Unknown cmd fd(3) cmd(c0046986){t:'i';sz:4} arg(ff90ed5c)
on /proc/i8k

I'll reboot/redo that again with strace shortly.

Re: Linux 3.3-rc5

[Mark Lord]

On 12-03-01 07:17 PM, Mark Lord wrote:
..
> Here's a go, 64-bit kernel (3.2.8), 32-bit userspace:
>
> Dell laptop SMM driver v1.14 21/02/2005 Massimo Dal Zotto (dz@debian.org)
> ioctl32(i8kctl:1474): Unknown cmd fd(3) cmd(80046980){t:'i';sz:4} arg(ff8b1fd8)
> on /proc/i8k
> ioctl32(i8kctl:1475): Unknown cmd fd(3) cmd(c0046986){t:'i';sz:4} arg(ff90ed5c)
> on /proc/i8k
> ioctl32(i8kctl:1475): Unknown cmd fd(3) cmd(c0046986){t:'i';sz:4} arg(ff90ed5c)
> on /proc/i8k
>
> I'll reboot/redo that again with strace shortly.

It all seems to work in 32-bit/32-bit mode, by the way.

Here's the promised strace from 64/32 mode:


$ strace -e raw=ioctl i8kctl fan

execve("/usr/bin/i8kctl", ["i8kctl", "fan"], [/* 21 vars */]) = 0
brk(0)                                  = 0x8e6a000
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
mmap2(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xf77c9000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY)      = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=123776, ...}) = 0
mmap2(NULL, 123776, PROT_READ, MAP_PRIVATE, 3, 0) = 0xf77aa000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/tls/i686/cmov/libc.so.6", O_RDONLY) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0000m\1\0004\0\0\0"...,
512) = 512
fstat64(3, {st_mode=S_IFREG|0755, st_size=1405508, ...}) = 0
mmap2(NULL, 1415592, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) =
0xf7650000
mprotect(0xf77a3000, 4096, PROT_NONE)   = 0
mmap2(0xf77a4000, 12288, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x153) = 0xf77a4000
mmap2(0xf77a7000, 10664, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xf77a7000
close(3)                                = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xf764f000
set_thread_area({entry_number:-1 -> 12, base_addr:0xf764f8d0, limit:1048575,
seg_32bit:1, contents:0, read_exec_only:0, limit_in_pages:1, seg_not_present:0,
useable:1}) = 0
mprotect(0xf77a4000, 8192, PROT_READ)   = 0
mprotect(0x804a000, 4096, PROT_READ)    = 0
mprotect(0xf77e7000, 4096, PROT_READ)   = 0
munmap(0xf77aa000, 123776)              = 0
open("/proc/i8k", O_RDONLY)             = 3
ioctl(0x3, 0xc0046986, 0xffc06dcc)      = -1 (errno 22)
ioctl(0x3, 0xc0046986, 0xffc06dcc)      = -1 (errno 22)
fstat64(1, {st_mode=S_IFREG|0644, st_size=1848, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xf77c8000
write(1, "-1 -1\n", 6-1 -1
)                  = 6
exit_group(0)                           = ?

and from dmesg:

ioctl32(i8kctl:1434): Unknown cmd fd(3) cmd(c0046986){t:'i';sz:4} arg(ffc06dcc)
on /proc/i8k
ioctl32(i8kctl:1434): Unknown cmd fd(3) cmd(c0046986){t:'i';sz:4} arg(ffc06dcc)
on /proc/i8k

Re: Linux 3.3-rc5

[Linus Torvalds]

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?

               Linus

Re: [PATCH] i8k: fix ioctl handing (was Re: Linux 3.3-rc5)

[Linus Torvalds]

On Thu, Mar 1, 2012 at 7:26 AM, Mark Lord <kernel@teksavvy.com> wrote:
>
> I'm still not 100% happy with the arg/argp passing to i8ki_unlocked_ioctl() though.
> Shouldn't there be a compat_ptr() macro in there someplace?

Yes, I think you should have "compat_ptr(arg)" there.

Patch looks good. And perhaps i8k_get_bios_version() should be renamed
to "i8k_get_smm_bios_version()" to not cause this crazy confusion
again.

                    Linus

Re: Linux 3.3-rc5

[Mark Lord]

[-- 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: