linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 01/11] compat_ioctl: remove keyboard ioctl translation
@ 2018-09-08 14:28 Arnd Bergmann
  2018-09-08 14:28 ` [PATCH 02/11] compat_ioctl: remove HIDIO translation Arnd Bergmann
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-08 14:28 UTC (permalink / raw)
  To: viro; +Cc: Arnd Bergmann, linux-fsdevel, linux-kernel

The KD* family of ioctls is implemented in two drivers:
drivers/tty/vt and drivers/s390/char/tty3270.c. Both of them
have compat handlers for all their ioctl commands, so translation
in fs/compat_ioctl.c is never used.

Commit fb07a5f857ac ("compat_ioctl: remove all VT ioctl handling")
removed the compat handling for all the other VT ioctls back in
2009, but it seems I missed the keyboard ones back then.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/compat_ioctl.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 9237076bdcf5..4c2f83a386a2 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -546,23 +546,6 @@ COMPATIBLE_IOCTL(FIGETBSZ)
 COMPATIBLE_IOCTL(FIFREEZE)
 COMPATIBLE_IOCTL(FITHAW)
 COMPATIBLE_IOCTL(FITRIM)
-COMPATIBLE_IOCTL(KDGETKEYCODE)
-COMPATIBLE_IOCTL(KDSETKEYCODE)
-COMPATIBLE_IOCTL(KDGKBTYPE)
-COMPATIBLE_IOCTL(KDGETMODE)
-COMPATIBLE_IOCTL(KDGKBMODE)
-COMPATIBLE_IOCTL(KDGKBMETA)
-COMPATIBLE_IOCTL(KDGKBENT)
-COMPATIBLE_IOCTL(KDSKBENT)
-COMPATIBLE_IOCTL(KDGKBSENT)
-COMPATIBLE_IOCTL(KDSKBSENT)
-COMPATIBLE_IOCTL(KDGKBDIACR)
-COMPATIBLE_IOCTL(KDSKBDIACR)
-COMPATIBLE_IOCTL(KDGKBDIACRUC)
-COMPATIBLE_IOCTL(KDSKBDIACRUC)
-COMPATIBLE_IOCTL(KDKBDREP)
-COMPATIBLE_IOCTL(KDGKBLED)
-COMPATIBLE_IOCTL(KDGETLED)
 #ifdef CONFIG_BLOCK
 /* Big S */
 COMPATIBLE_IOCTL(SCSI_IOCTL_GET_IDLUN)
@@ -974,15 +957,6 @@ static long do_ioctl_trans(unsigned int cmd,
 	case HOT_ADD_DISK:
 	case SET_DISK_FAULTY:
 	case SET_BITMAP_FILE:
-	/* Big K */
-	case KDSIGACCEPT:
-	case KIOCSOUND:
-	case KDMKTONE:
-	case KDSETMODE:
-	case KDSKBMODE:
-	case KDSKBMETA:
-	case KDSKBLED:
-	case KDSETLED:
 		return vfs_ioctl(file, cmd, arg);
 	}
 
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 02/11] compat_ioctl: remove HIDIO translation
  2018-09-08 14:28 [PATCH 01/11] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann
@ 2018-09-08 14:28 ` Arnd Bergmann
  2018-09-08 14:28 ` [PATCH 03/11] compat_ioctl: remove translation for sound ioctls Arnd Bergmann
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-08 14:28 UTC (permalink / raw)
  To: viro; +Cc: Arnd Bergmann, linux-fsdevel, linux-kernel

The two drivers implementing these both gained proper compat_ioctl()
handlers a long time ago with commits bb6c8d8fa9b5 ("HID: hiddev:
Add 32bit ioctl compatibilty") and ae5e49c79c05 ("HID: hidraw: add
compatibility ioctl() for 32-bit applications."), so the lists in
fs/compat_ioctl.c are no longer used.

It appears that the lists were also incomplete, so the translation
didn't actually work correctly when it was still in use.

Remove them as cleanup.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/compat_ioctl.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 4c2f83a386a2..c2c73b802fb3 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -853,23 +853,6 @@ COMPATIBLE_IOCTL(PCIIOC_CONTROLLER)
 COMPATIBLE_IOCTL(PCIIOC_MMAP_IS_IO)
 COMPATIBLE_IOCTL(PCIIOC_MMAP_IS_MEM)
 COMPATIBLE_IOCTL(PCIIOC_WRITE_COMBINE)
-/* hiddev */
-COMPATIBLE_IOCTL(HIDIOCGVERSION)
-COMPATIBLE_IOCTL(HIDIOCAPPLICATION)
-COMPATIBLE_IOCTL(HIDIOCGDEVINFO)
-COMPATIBLE_IOCTL(HIDIOCGSTRING)
-COMPATIBLE_IOCTL(HIDIOCINITREPORT)
-COMPATIBLE_IOCTL(HIDIOCGREPORT)
-COMPATIBLE_IOCTL(HIDIOCSREPORT)
-COMPATIBLE_IOCTL(HIDIOCGREPORTINFO)
-COMPATIBLE_IOCTL(HIDIOCGFIELDINFO)
-COMPATIBLE_IOCTL(HIDIOCGUSAGE)
-COMPATIBLE_IOCTL(HIDIOCSUSAGE)
-COMPATIBLE_IOCTL(HIDIOCGUCODE)
-COMPATIBLE_IOCTL(HIDIOCGFLAG)
-COMPATIBLE_IOCTL(HIDIOCSFLAG)
-COMPATIBLE_IOCTL(HIDIOCGCOLLECTIONINDEX)
-COMPATIBLE_IOCTL(HIDIOCGCOLLECTIONINFO)
 /* joystick */
 COMPATIBLE_IOCTL(JSIOCGVERSION)
 COMPATIBLE_IOCTL(JSIOCGAXES)
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 03/11] compat_ioctl: remove translation for sound ioctls
  2018-09-08 14:28 [PATCH 01/11] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann
  2018-09-08 14:28 ` [PATCH 02/11] compat_ioctl: remove HIDIO translation Arnd Bergmann
@ 2018-09-08 14:28 ` Arnd Bergmann
  2018-09-09  4:16   ` Al Viro
  2018-09-08 14:28 ` [PATCH 04/11] compat_ioctl: remove isdn ioctl translation Arnd Bergmann
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-08 14:28 UTC (permalink / raw)
  To: viro, Jeff Dike, Richard Weinberger, Jaroslav Kysela,
	Takashi Iwai, Arnd Bergmann
  Cc: linux-um, linux-kernel, linux-fsdevel, alsa-devel

The SNDCTL_* and SOUND_* commands are the old OSS user interface.

I checked all the sound ioctl commands listed in fs/compat_ioctl.c
to see if we still need the translation handlers. Here is what I
found:

- sound/oss/ is (almost) gone from the kernel, this is what actually
  needed all the translations
- The ALSA emulation for OSS correctly handles all compat_ioctl
  commands already.
- sound/oss/dmasound/ is the last holdout of the original OSS code,
  this is only used on arch/m68k, which has no 64-bit mode and
  hence needs no compat handlers
- arch/um/drivers/hostaudio_kern.c may run in 64-bit mode with
  32-bit x86 user space underneath it. This rare corner case is
  the only one that still needs the compat handlers.

By adding a simple redirect of .compat_ioctl to .unlocked_ioctl in the
UML driver, we can remove all the COMPATIBLE_IOCTL() annotations without
a change in functionality. For completeness, I'm adding the same thing
to the dmasound file, knowing that it makes no difference.

The compat_ioctl list contains one comment about SNDCTL_DSP_MAPINBUF and
SNDCTL_DSP_MAPOUTBUF, which actually would need a translation handler
if implemented. However, the native implementation just returns -EINVAL,
so we don't care.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/um/drivers/hostaudio_kern.c   |   1 +
 fs/compat_ioctl.c                  | 157 -----------------------------
 sound/core/oss/pcm_oss.c           |   4 +
 sound/oss/dmasound/dmasound_core.c |   2 +
 4 files changed, 7 insertions(+), 157 deletions(-)

diff --git a/arch/um/drivers/hostaudio_kern.c b/arch/um/drivers/hostaudio_kern.c
index 7f9dbdbc4eb7..0278a642a622 100644
--- a/arch/um/drivers/hostaudio_kern.c
+++ b/arch/um/drivers/hostaudio_kern.c
@@ -298,6 +298,7 @@ static const struct file_operations hostaudio_fops = {
 	.write          = hostaudio_write,
 	.poll           = hostaudio_poll,
 	.unlocked_ioctl	= hostaudio_ioctl,
+	.compat_ioctl	= hostaudio_ioctl,
 	.mmap           = NULL,
 	.open           = hostaudio_open,
 	.release        = hostaudio_release,
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index c2c73b802fb3..875516658c39 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -78,7 +78,6 @@
 #include <linux/if_bonding.h>
 #include <linux/watchdog.h>
 
-#include <linux/soundcard.h>
 #include <linux/lp.h>
 #include <linux/ppdev.h>
 
@@ -605,162 +604,6 @@ COMPATIBLE_IOCTL(SG_GET_KEEP_ORPHAN)
 /* PPP stuff */
 COMPATIBLE_IOCTL(PPPIOCGUNIT)
 COMPATIBLE_IOCTL(PPPIOCGCHAN)
-/* Big A */
-/* sparc only */
-/* Big Q for sound/OSS */
-COMPATIBLE_IOCTL(SNDCTL_SEQ_RESET)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_SYNC)
-COMPATIBLE_IOCTL(SNDCTL_SYNTH_INFO)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_CTRLRATE)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_GETOUTCOUNT)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_GETINCOUNT)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_PERCMODE)
-COMPATIBLE_IOCTL(SNDCTL_FM_LOAD_INSTR)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_TESTMIDI)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_RESETSAMPLES)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_NRSYNTHS)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_NRMIDIS)
-COMPATIBLE_IOCTL(SNDCTL_MIDI_INFO)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_THRESHOLD)
-COMPATIBLE_IOCTL(SNDCTL_SYNTH_MEMAVL)
-COMPATIBLE_IOCTL(SNDCTL_FM_4OP_ENABLE)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_PANIC)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_OUTOFBAND)
-COMPATIBLE_IOCTL(SNDCTL_SEQ_GETTIME)
-COMPATIBLE_IOCTL(SNDCTL_SYNTH_ID)
-COMPATIBLE_IOCTL(SNDCTL_SYNTH_CONTROL)
-COMPATIBLE_IOCTL(SNDCTL_SYNTH_REMOVESAMPLE)
-/* Big T for sound/OSS */
-COMPATIBLE_IOCTL(SNDCTL_TMR_TIMEBASE)
-COMPATIBLE_IOCTL(SNDCTL_TMR_START)
-COMPATIBLE_IOCTL(SNDCTL_TMR_STOP)
-COMPATIBLE_IOCTL(SNDCTL_TMR_CONTINUE)
-COMPATIBLE_IOCTL(SNDCTL_TMR_TEMPO)
-COMPATIBLE_IOCTL(SNDCTL_TMR_SOURCE)
-COMPATIBLE_IOCTL(SNDCTL_TMR_METRONOME)
-COMPATIBLE_IOCTL(SNDCTL_TMR_SELECT)
-/* Little m for sound/OSS */
-COMPATIBLE_IOCTL(SNDCTL_MIDI_PRETIME)
-COMPATIBLE_IOCTL(SNDCTL_MIDI_MPUMODE)
-COMPATIBLE_IOCTL(SNDCTL_MIDI_MPUCMD)
-/* Big P for sound/OSS */
-COMPATIBLE_IOCTL(SNDCTL_DSP_RESET)
-COMPATIBLE_IOCTL(SNDCTL_DSP_SYNC)
-COMPATIBLE_IOCTL(SNDCTL_DSP_SPEED)
-COMPATIBLE_IOCTL(SNDCTL_DSP_STEREO)
-COMPATIBLE_IOCTL(SNDCTL_DSP_GETBLKSIZE)
-COMPATIBLE_IOCTL(SNDCTL_DSP_CHANNELS)
-COMPATIBLE_IOCTL(SOUND_PCM_WRITE_FILTER)
-COMPATIBLE_IOCTL(SNDCTL_DSP_POST)
-COMPATIBLE_IOCTL(SNDCTL_DSP_SUBDIVIDE)
-COMPATIBLE_IOCTL(SNDCTL_DSP_SETFRAGMENT)
-COMPATIBLE_IOCTL(SNDCTL_DSP_GETFMTS)
-COMPATIBLE_IOCTL(SNDCTL_DSP_SETFMT)
-COMPATIBLE_IOCTL(SNDCTL_DSP_GETOSPACE)
-COMPATIBLE_IOCTL(SNDCTL_DSP_GETISPACE)
-COMPATIBLE_IOCTL(SNDCTL_DSP_NONBLOCK)
-COMPATIBLE_IOCTL(SNDCTL_DSP_GETCAPS)
-COMPATIBLE_IOCTL(SNDCTL_DSP_GETTRIGGER)
-COMPATIBLE_IOCTL(SNDCTL_DSP_SETTRIGGER)
-COMPATIBLE_IOCTL(SNDCTL_DSP_GETIPTR)
-COMPATIBLE_IOCTL(SNDCTL_DSP_GETOPTR)
-/* SNDCTL_DSP_MAPINBUF,  XXX needs translation */
-/* SNDCTL_DSP_MAPOUTBUF,  XXX needs translation */
-COMPATIBLE_IOCTL(SNDCTL_DSP_SETSYNCRO)
-COMPATIBLE_IOCTL(SNDCTL_DSP_SETDUPLEX)
-COMPATIBLE_IOCTL(SNDCTL_DSP_GETODELAY)
-COMPATIBLE_IOCTL(SNDCTL_DSP_PROFILE)
-COMPATIBLE_IOCTL(SOUND_PCM_READ_RATE)
-COMPATIBLE_IOCTL(SOUND_PCM_READ_CHANNELS)
-COMPATIBLE_IOCTL(SOUND_PCM_READ_BITS)
-COMPATIBLE_IOCTL(SOUND_PCM_READ_FILTER)
-/* Big C for sound/OSS */
-COMPATIBLE_IOCTL(SNDCTL_COPR_RESET)
-COMPATIBLE_IOCTL(SNDCTL_COPR_LOAD)
-COMPATIBLE_IOCTL(SNDCTL_COPR_RDATA)
-COMPATIBLE_IOCTL(SNDCTL_COPR_RCODE)
-COMPATIBLE_IOCTL(SNDCTL_COPR_WDATA)
-COMPATIBLE_IOCTL(SNDCTL_COPR_WCODE)
-COMPATIBLE_IOCTL(SNDCTL_COPR_RUN)
-COMPATIBLE_IOCTL(SNDCTL_COPR_HALT)
-COMPATIBLE_IOCTL(SNDCTL_COPR_SENDMSG)
-COMPATIBLE_IOCTL(SNDCTL_COPR_RCVMSG)
-/* Big M for sound/OSS */
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_VOLUME)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_BASS)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_TREBLE)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_SYNTH)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_PCM)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_SPEAKER)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_LINE)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_MIC)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_CD)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_IMIX)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_ALTPCM)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_RECLEV)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_IGAIN)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_OGAIN)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_LINE1)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_LINE2)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_LINE3)
-COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_DIGITAL1))
-COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_DIGITAL2))
-COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_DIGITAL3))
-COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_PHONEIN))
-COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_PHONEOUT))
-COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_VIDEO))
-COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_RADIO))
-COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_MONITOR))
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_MUTE)
-/* SOUND_MIXER_READ_ENHANCE,  same value as READ_MUTE */
-/* SOUND_MIXER_READ_LOUD,  same value as READ_MUTE */
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_RECSRC)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_DEVMASK)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_RECMASK)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_STEREODEVS)
-COMPATIBLE_IOCTL(SOUND_MIXER_READ_CAPS)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_VOLUME)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_BASS)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_TREBLE)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_SYNTH)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_PCM)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_SPEAKER)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_LINE)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_MIC)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_CD)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_IMIX)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_ALTPCM)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_RECLEV)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_IGAIN)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_OGAIN)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_LINE1)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_LINE2)
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_LINE3)
-COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_DIGITAL1))
-COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_DIGITAL2))
-COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_DIGITAL3))
-COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_PHONEIN))
-COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_PHONEOUT))
-COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_VIDEO))
-COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_RADIO))
-COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_MONITOR))
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_MUTE)
-/* SOUND_MIXER_WRITE_ENHANCE,  same value as WRITE_MUTE */
-/* SOUND_MIXER_WRITE_LOUD,  same value as WRITE_MUTE */
-COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_RECSRC)
-COMPATIBLE_IOCTL(SOUND_MIXER_INFO)
-COMPATIBLE_IOCTL(SOUND_OLD_MIXER_INFO)
-COMPATIBLE_IOCTL(SOUND_MIXER_ACCESS)
-COMPATIBLE_IOCTL(SOUND_MIXER_AGC)
-COMPATIBLE_IOCTL(SOUND_MIXER_3DSE)
-COMPATIBLE_IOCTL(SOUND_MIXER_PRIVATE1)
-COMPATIBLE_IOCTL(SOUND_MIXER_PRIVATE2)
-COMPATIBLE_IOCTL(SOUND_MIXER_PRIVATE3)
-COMPATIBLE_IOCTL(SOUND_MIXER_PRIVATE4)
-COMPATIBLE_IOCTL(SOUND_MIXER_PRIVATE5)
-COMPATIBLE_IOCTL(SOUND_MIXER_GETLEVELS)
-COMPATIBLE_IOCTL(SOUND_MIXER_SETLEVELS)
-COMPATIBLE_IOCTL(OSS_GETVERSION)
 /* Raw devices */
 COMPATIBLE_IOCTL(RAW_SETBIND)
 COMPATIBLE_IOCTL(RAW_GETBIND)
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index f8d4a419f3af..3f25c9d7b7df 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -2732,6 +2732,10 @@ static long snd_pcm_oss_ioctl(struct file *file, unsigned int cmd, unsigned long
 static long snd_pcm_oss_ioctl_compat(struct file *file, unsigned int cmd,
 				     unsigned long arg)
 {
+	/*
+	 * Everything is compatbile except SNDCTL_DSP_MAPINBUF/SNDCTL_DSP_MAPOUTBUF,
+	 * which are not implemented for the native case either
+	 */
 	return snd_pcm_oss_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
 }
 #else
diff --git a/sound/oss/dmasound/dmasound_core.c b/sound/oss/dmasound/dmasound_core.c
index fc9bcd47d6a4..b6b5f5a5df9c 100644
--- a/sound/oss/dmasound/dmasound_core.c
+++ b/sound/oss/dmasound/dmasound_core.c
@@ -384,6 +384,7 @@ static const struct file_operations mixer_fops =
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
 	.unlocked_ioctl	= mixer_unlocked_ioctl,
+	.compat_ioctl	= mixer_unlocked_ioctl,
 	.open		= mixer_open,
 	.release	= mixer_release,
 };
@@ -1167,6 +1168,7 @@ static const struct file_operations sq_fops =
 	.write		= sq_write,
 	.poll		= sq_poll,
 	.unlocked_ioctl	= sq_unlocked_ioctl,
+	.compat_ioctl	= sq_unlocked_ioctl,
 	.open		= sq_open,
 	.release	= sq_release,
 };
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 04/11] compat_ioctl: remove isdn ioctl translation
  2018-09-08 14:28 [PATCH 01/11] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann
  2018-09-08 14:28 ` [PATCH 02/11] compat_ioctl: remove HIDIO translation Arnd Bergmann
  2018-09-08 14:28 ` [PATCH 03/11] compat_ioctl: remove translation for sound ioctls Arnd Bergmann
@ 2018-09-08 14:28 ` Arnd Bergmann
  2018-09-08 14:28 ` [PATCH 05/11] compat_ioctl: remove IGNORE_IOCTL() Arnd Bergmann
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-08 14:28 UTC (permalink / raw)
  To: viro
  Cc: Arnd Bergmann, Karsten Keil, Paul Bolle, netdev, linux-kernel,
	gigaset307x-common, linux-fsdevel

Neither the old isdn4linux interface nor the newer mISDN stack
ever had working 32-bit compat mode as far as I can tell.

However, the CAPI stack and the Gigaset ISDN driver (implemented
on top of either CAPI or i4l) have some ioctl commands that are
correctly listed in fs/compat_ioctl.c.

We can trivially move all of those into the two corresponding
files that implement the native handlers by adding a compat_ioctl
redirect to that.

I did notice that treating CAPI_MANUFACTURER_CMD() as compatible
is broken, so I'm also adding a handler for that, realizing that
in all likelyhood, nobody is ever going to call it.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/isdn/capi/capi.c         | 31 +++++++++++++++++++++++++++++++
 drivers/isdn/gigaset/interface.c | 10 ++++++++++
 fs/compat_ioctl.c                | 22 ----------------------
 3 files changed, 41 insertions(+), 22 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index ef5560b848ab..4851dc3941eb 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -942,6 +942,34 @@ capi_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return ret;
 }
 
+#ifdef CONFIG_COMPAT
+static long
+capi_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	int ret;
+
+	if (cmd == CAPI_MANUFACTURER_CMD) {
+		struct {
+			unsigned long cmd;
+			compat_uptr_t data;
+		} mcmd32;
+
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+		if (copy_from_user(&mcmd32, compat_ptr(arg), sizeof(mcmd32)))
+			return -EFAULT;
+
+		mutex_lock(&capi_mutex);
+		ret = capi20_manufacturer(mcmd32.cmd, compat_ptr(mcmd32.data));
+		mutex_unlock(&capi_mutex);
+
+		return ret;
+	}
+
+	return capi_unlocked_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
+}
+#endif
+
 static int capi_open(struct inode *inode, struct file *file)
 {
 	struct capidev *cdev;
@@ -988,6 +1016,9 @@ static const struct file_operations capi_fops =
 	.write		= capi_write,
 	.poll		= capi_poll,
 	.unlocked_ioctl	= capi_unlocked_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= capi_compat_ioctl,
+#endif
 	.open		= capi_open,
 	.release	= capi_release,
 };
diff --git a/drivers/isdn/gigaset/interface.c b/drivers/isdn/gigaset/interface.c
index 600c79b030cd..faae8ae54802 100644
--- a/drivers/isdn/gigaset/interface.c
+++ b/drivers/isdn/gigaset/interface.c
@@ -233,6 +233,13 @@ static int if_ioctl(struct tty_struct *tty,
 	return retval;
 }
 
+#ifdef CONFIG_COMPAT
+static long if_compat_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
+{
+	return if_ioctl(tty, cmd, (unsigned long)compat_ptr(arg));
+}
+#endif
+
 static int if_tiocmget(struct tty_struct *tty)
 {
 	struct cardstate *cs = tty->driver_data;
@@ -472,6 +479,9 @@ static const struct tty_operations if_ops = {
 	.open =			if_open,
 	.close =		if_close,
 	.ioctl =		if_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl =		if_compat_ioctl,
+#endif
 	.write =		if_write,
 	.write_room =		if_write_room,
 	.chars_in_buffer =	if_chars_in_buffer,
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 875516658c39..7d8c5722fd0a 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -61,9 +61,6 @@
 #include <net/bluetooth/hci_sock.h>
 #include <net/bluetooth/rfcomm.h>
 
-#include <linux/capi.h>
-#include <linux/gigaset_dev.h>
-
 #ifdef CONFIG_BLOCK
 #include <linux/cdrom.h>
 #include <linux/fd.h>
@@ -670,25 +667,6 @@ COMPATIBLE_IOCTL(HIDPCONNADD)
 COMPATIBLE_IOCTL(HIDPCONNDEL)
 COMPATIBLE_IOCTL(HIDPGETCONNLIST)
 COMPATIBLE_IOCTL(HIDPGETCONNINFO)
-/* CAPI */
-COMPATIBLE_IOCTL(CAPI_REGISTER)
-COMPATIBLE_IOCTL(CAPI_GET_MANUFACTURER)
-COMPATIBLE_IOCTL(CAPI_GET_VERSION)
-COMPATIBLE_IOCTL(CAPI_GET_SERIAL)
-COMPATIBLE_IOCTL(CAPI_GET_PROFILE)
-COMPATIBLE_IOCTL(CAPI_MANUFACTURER_CMD)
-COMPATIBLE_IOCTL(CAPI_GET_ERRCODE)
-COMPATIBLE_IOCTL(CAPI_INSTALLED)
-COMPATIBLE_IOCTL(CAPI_GET_FLAGS)
-COMPATIBLE_IOCTL(CAPI_SET_FLAGS)
-COMPATIBLE_IOCTL(CAPI_CLR_FLAGS)
-COMPATIBLE_IOCTL(CAPI_NCCI_OPENCOUNT)
-COMPATIBLE_IOCTL(CAPI_NCCI_GETUNIT)
-/* Siemens Gigaset */
-COMPATIBLE_IOCTL(GIGASET_REDIR)
-COMPATIBLE_IOCTL(GIGASET_CONFIG)
-COMPATIBLE_IOCTL(GIGASET_BRKCHARS)
-COMPATIBLE_IOCTL(GIGASET_VERSION)
 /* Misc. */
 COMPATIBLE_IOCTL(0x41545900)		/* ATYIO_CLKR */
 COMPATIBLE_IOCTL(0x41545901)		/* ATYIO_CLKW */
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 05/11] compat_ioctl: remove IGNORE_IOCTL()
  2018-09-08 14:28 [PATCH 01/11] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann
                   ` (2 preceding siblings ...)
  2018-09-08 14:28 ` [PATCH 04/11] compat_ioctl: remove isdn ioctl translation Arnd Bergmann
@ 2018-09-08 14:28 ` Arnd Bergmann
  2018-09-08 14:28 ` [PATCH 06/11] compat_ioctl: remove /dev/random commands Arnd Bergmann
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-08 14:28 UTC (permalink / raw)
  To: viro; +Cc: Arnd Bergmann, linux-fsdevel, linux-kernel

Since commit 07d106d0a33d ("vfs: fix up ENOIOCTLCMD error handling"),
we don't warn about unhandled compat-ioctl command code any more, but
just return the same error that a native file descriptor returns when
there is no handler.

This means the IGNORE_IOCTL() annotations are completely useless and
can all be removed. TIOCSTART/TIOCSTOP and KDGHWCLK/KDSHWCLK fall into
the same category, but for some reason were listed as COMPATIBLE_IOCTL().

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/compat_ioctl.c | 56 -----------------------------------------------
 1 file changed, 56 deletions(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 7d8c5722fd0a..b56a3842d61d 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -463,20 +463,7 @@ static int compat_ioctl_preallocate(struct file *file,
 #define XFORM(i) (((i) ^ ((i) << 27) ^ ((i) << 17)) & 0xffffffff)
 
 #define COMPATIBLE_IOCTL(cmd) XFORM((u32)cmd),
-/* ioctl should not be warned about even if it's not implemented.
-   Valid reasons to use this:
-   - It is implemented with ->compat_ioctl on some device, but programs
-   call it on others too.
-   - The ioctl is not implemented in the native kernel, but programs
-   call it commonly anyways.
-   Most other reasons are not valid. */
-#define IGNORE_IOCTL(cmd) COMPATIBLE_IOCTL(cmd)
-
 static unsigned int ioctl_pointer[] = {
-/* compatible ioctls first */
-COMPATIBLE_IOCTL(0x4B50)   /* KDGHWCLK - not in the kernel, but don't complain */
-COMPATIBLE_IOCTL(0x4B51)   /* KDSHWCLK - not in the kernel, but don't complain */
-
 /* Big T */
 COMPATIBLE_IOCTL(TCGETA)
 COMPATIBLE_IOCTL(TCSETA)
@@ -553,9 +540,6 @@ COMPATIBLE_IOCTL(SCSI_IOCTL_SEND_COMMAND)
 COMPATIBLE_IOCTL(SCSI_IOCTL_PROBE_HOST)
 COMPATIBLE_IOCTL(SCSI_IOCTL_GET_PCI)
 #endif
-/* Big V (don't complain on serial console) */
-IGNORE_IOCTL(VT_OPENQRY)
-IGNORE_IOCTL(VT_GETMODE)
 /*
  * These two are only for the sbus rtc driver, but
  * hwclock tries them on every rtc device first when
@@ -569,11 +553,6 @@ COMPATIBLE_IOCTL(MTIOCTOP)
 /* Socket level stuff */
 COMPATIBLE_IOCTL(FIOQSIZE)
 #ifdef CONFIG_BLOCK
-/* md calls this on random blockdevs */
-IGNORE_IOCTL(RAID_VERSION)
-/* qemu/qemu-img might call these two on plain files for probing */
-IGNORE_IOCTL(CDROM_DRIVE_STATUS)
-IGNORE_IOCTL(FDGETPRM32)
 /* SG stuff */
 COMPATIBLE_IOCTL(SG_SET_TIMEOUT)
 COMPATIBLE_IOCTL(SG_GET_TIMEOUT)
@@ -684,41 +663,6 @@ COMPATIBLE_IOCTL(JSIOCGNAME(0))
 COMPATIBLE_IOCTL(TIOCGLTC)
 COMPATIBLE_IOCTL(TIOCSLTC)
 #endif
-#ifdef TIOCSTART
-/*
- * For these two we have definitions in ioctls.h and/or termios.h on
- * some architectures but no actual implemention.  Some applications
- * like bash call them if they are defined in the headers, so we provide
- * entries here to avoid syslog message spew.
- */
-COMPATIBLE_IOCTL(TIOCSTART)
-COMPATIBLE_IOCTL(TIOCSTOP)
-#endif
-
-/* fat 'r' ioctls. These are handled by fat with ->compat_ioctl,
-   but we don't want warnings on other file systems. So declare
-   them as compatible here. */
-#define VFAT_IOCTL_READDIR_BOTH32       _IOR('r', 1, struct compat_dirent[2])
-#define VFAT_IOCTL_READDIR_SHORT32      _IOR('r', 2, struct compat_dirent[2])
-
-IGNORE_IOCTL(VFAT_IOCTL_READDIR_BOTH32)
-IGNORE_IOCTL(VFAT_IOCTL_READDIR_SHORT32)
-
-#ifdef CONFIG_SPARC
-/* Sparc framebuffers, handled in sbusfb_compat_ioctl() */
-IGNORE_IOCTL(FBIOGTYPE)
-IGNORE_IOCTL(FBIOSATTR)
-IGNORE_IOCTL(FBIOGATTR)
-IGNORE_IOCTL(FBIOSVIDEO)
-IGNORE_IOCTL(FBIOGVIDEO)
-IGNORE_IOCTL(FBIOSCURPOS)
-IGNORE_IOCTL(FBIOGCURPOS)
-IGNORE_IOCTL(FBIOGCURMAX)
-IGNORE_IOCTL(FBIOPUTCMAP32)
-IGNORE_IOCTL(FBIOGETCMAP32)
-IGNORE_IOCTL(FBIOSCURSOR32)
-IGNORE_IOCTL(FBIOGCURSOR32)
-#endif
 };
 
 /*
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 06/11] compat_ioctl: remove /dev/random commands
  2018-09-08 14:28 [PATCH 01/11] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann
                   ` (3 preceding siblings ...)
  2018-09-08 14:28 ` [PATCH 05/11] compat_ioctl: remove IGNORE_IOCTL() Arnd Bergmann
@ 2018-09-08 14:28 ` Arnd Bergmann
  2018-09-09  4:11   ` Al Viro
  2018-09-08 14:28 ` [PATCH 07/11] compat_ioctl: remove joystick ioctl translation Arnd Bergmann
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-08 14:28 UTC (permalink / raw)
  To: viro
  Cc: Arnd Bergmann, Theodore Ts'o, Greg Kroah-Hartman,
	linux-kernel, linux-fsdevel

These are all handled by the random driver, so instead of listing
each ioctl, we can just use the same function to deal with both
native and compat commands.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/char/random.c | 1 +
 fs/compat_ioctl.c     | 7 -------
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index bf5f99fc36f1..103abf82444a 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -2021,6 +2021,7 @@ const struct file_operations random_fops = {
 	.write = random_write,
 	.poll  = random_poll,
 	.unlocked_ioctl = random_ioctl,
+	.compat_ioctl = random_ioctl,
 	.fasync = random_fasync,
 	.llseek = noop_llseek,
 };
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index b56a3842d61d..eb29188d1dbb 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -594,13 +594,6 @@ COMPATIBLE_IOCTL(WDIOC_SETTIMEOUT)
 COMPATIBLE_IOCTL(WDIOC_GETTIMEOUT)
 COMPATIBLE_IOCTL(WDIOC_SETPRETIMEOUT)
 COMPATIBLE_IOCTL(WDIOC_GETPRETIMEOUT)
-/* Big R */
-COMPATIBLE_IOCTL(RNDGETENTCNT)
-COMPATIBLE_IOCTL(RNDADDTOENTCNT)
-COMPATIBLE_IOCTL(RNDGETPOOL)
-COMPATIBLE_IOCTL(RNDADDENTROPY)
-COMPATIBLE_IOCTL(RNDZAPENTCNT)
-COMPATIBLE_IOCTL(RNDCLEARPOOL)
 /* Bluetooth */
 COMPATIBLE_IOCTL(HCIDEVUP)
 COMPATIBLE_IOCTL(HCIDEVDOWN)
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 07/11] compat_ioctl: remove joystick ioctl translation
  2018-09-08 14:28 [PATCH 01/11] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann
                   ` (4 preceding siblings ...)
  2018-09-08 14:28 ` [PATCH 06/11] compat_ioctl: remove /dev/random commands Arnd Bergmann
@ 2018-09-08 14:28 ` Arnd Bergmann
  2018-09-08 14:28 ` [PATCH 08/11] compat_ioctl: remove PCI " Arnd Bergmann
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-08 14:28 UTC (permalink / raw)
  To: viro; +Cc: Arnd Bergmann, linux-fsdevel, linux-kernel

The joystick driver already handles these just fine, so
the entries in the table are not needed any more.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/compat_ioctl.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index eb29188d1dbb..23c0406d8b30 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -11,8 +11,6 @@
  * ioctls.
  */
 
-#include <linux/joystick.h>
-
 #include <linux/types.h>
 #include <linux/compat.h>
 #include <linux/kernel.h>
@@ -646,12 +644,6 @@ COMPATIBLE_IOCTL(PCIIOC_CONTROLLER)
 COMPATIBLE_IOCTL(PCIIOC_MMAP_IS_IO)
 COMPATIBLE_IOCTL(PCIIOC_MMAP_IS_MEM)
 COMPATIBLE_IOCTL(PCIIOC_WRITE_COMBINE)
-/* joystick */
-COMPATIBLE_IOCTL(JSIOCGVERSION)
-COMPATIBLE_IOCTL(JSIOCGAXES)
-COMPATIBLE_IOCTL(JSIOCGBUTTONS)
-COMPATIBLE_IOCTL(JSIOCGNAME(0))
-
 #ifdef TIOCGLTC
 COMPATIBLE_IOCTL(TIOCGLTC)
 COMPATIBLE_IOCTL(TIOCSLTC)
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 08/11] compat_ioctl: remove PCI ioctl translation
  2018-09-08 14:28 [PATCH 01/11] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann
                   ` (5 preceding siblings ...)
  2018-09-08 14:28 ` [PATCH 07/11] compat_ioctl: remove joystick ioctl translation Arnd Bergmann
@ 2018-09-08 14:28 ` Arnd Bergmann
  2018-09-08 14:28 ` [PATCH 09/11] compat_ioctl: remove /dev/raw " Arnd Bergmann
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-08 14:28 UTC (permalink / raw)
  To: viro; +Cc: Arnd Bergmann, linux-fsdevel, linux-kernel

The /proc/pci/ implementation already handles these just fine, so
the entries in the table are not needed any more.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/compat_ioctl.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 23c0406d8b30..28ebff2d1799 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -44,7 +44,6 @@
 #include <linux/raw.h>
 #include <linux/blkdev.h>
 #include <linux/elevator.h>
-#include <linux/pci.h>
 #include <linux/serial.h>
 #include <linux/if_tun.h>
 #include <linux/ctype.h>
@@ -640,10 +639,6 @@ COMPATIBLE_IOCTL(HIDPGETCONNINFO)
 /* Misc. */
 COMPATIBLE_IOCTL(0x41545900)		/* ATYIO_CLKR */
 COMPATIBLE_IOCTL(0x41545901)		/* ATYIO_CLKW */
-COMPATIBLE_IOCTL(PCIIOC_CONTROLLER)
-COMPATIBLE_IOCTL(PCIIOC_MMAP_IS_IO)
-COMPATIBLE_IOCTL(PCIIOC_MMAP_IS_MEM)
-COMPATIBLE_IOCTL(PCIIOC_WRITE_COMBINE)
 #ifdef TIOCGLTC
 COMPATIBLE_IOCTL(TIOCGLTC)
 COMPATIBLE_IOCTL(TIOCSLTC)
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 09/11] compat_ioctl: remove /dev/raw ioctl translation
  2018-09-08 14:28 [PATCH 01/11] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann
                   ` (6 preceding siblings ...)
  2018-09-08 14:28 ` [PATCH 08/11] compat_ioctl: remove PCI " Arnd Bergmann
@ 2018-09-08 14:28 ` Arnd Bergmann
  2018-09-08 14:28 ` [PATCH 10/11] compat_ioctl: remove last RAID handling code Arnd Bergmann
  2018-09-08 14:28 ` [PATCH 11/11] compat_ioctl: move tape handling into drivers Arnd Bergmann
  9 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-08 14:28 UTC (permalink / raw)
  To: viro; +Cc: Arnd Bergmann, linux-fsdevel, linux-kernel

The /dev/rawX implementation already handles these just fine, so
the entries in the table are not needed any more.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/compat_ioctl.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 28ebff2d1799..e755802cd887 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -41,7 +41,6 @@
 #include <linux/fb.h>
 #include <linux/videodev2.h>
 #include <linux/netdevice.h>
-#include <linux/raw.h>
 #include <linux/blkdev.h>
 #include <linux/elevator.h>
 #include <linux/serial.h>
@@ -577,9 +576,6 @@ COMPATIBLE_IOCTL(SG_GET_KEEP_ORPHAN)
 /* PPP stuff */
 COMPATIBLE_IOCTL(PPPIOCGUNIT)
 COMPATIBLE_IOCTL(PPPIOCGCHAN)
-/* Raw devices */
-COMPATIBLE_IOCTL(RAW_SETBIND)
-COMPATIBLE_IOCTL(RAW_GETBIND)
 /* Watchdog */
 COMPATIBLE_IOCTL(WDIOC_GETSUPPORT)
 COMPATIBLE_IOCTL(WDIOC_GETSTATUS)
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 10/11] compat_ioctl: remove last RAID handling code
  2018-09-08 14:28 [PATCH 01/11] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann
                   ` (7 preceding siblings ...)
  2018-09-08 14:28 ` [PATCH 09/11] compat_ioctl: remove /dev/raw " Arnd Bergmann
@ 2018-09-08 14:28 ` Arnd Bergmann
  2018-09-08 14:28 ` [PATCH 11/11] compat_ioctl: move tape handling into drivers Arnd Bergmann
  9 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-08 14:28 UTC (permalink / raw)
  To: viro; +Cc: Arnd Bergmann, linux-fsdevel, linux-kernel

Commit aa98aa31987a ("md: move compat_ioctl handling into md.c")
already removed the COMPATIBLE_IOCTL() table entries and added
a complete implementation, but a few lines got left behind and
should also be removed here.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/compat_ioctl.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index e755802cd887..bb73d52f02a2 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -676,11 +676,6 @@ static long do_ioctl_trans(unsigned int cmd,
 	case TCSBRKP:
 	case TIOCMIWAIT:
 	case TIOCSCTTY:
-	/* RAID */
-	case HOT_REMOVE_DISK:
-	case HOT_ADD_DISK:
-	case SET_DISK_FAULTY:
-	case SET_BITMAP_FILE:
 		return vfs_ioctl(file, cmd, arg);
 	}
 
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 11/11] compat_ioctl: move tape handling into drivers
  2018-09-08 14:28 [PATCH 01/11] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann
                   ` (8 preceding siblings ...)
  2018-09-08 14:28 ` [PATCH 10/11] compat_ioctl: remove last RAID handling code Arnd Bergmann
@ 2018-09-08 14:28 ` Arnd Bergmann
  2018-09-09  4:37   ` Al Viro
  9 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-08 14:28 UTC (permalink / raw)
  To: viro
  Cc: Arnd Bergmann, David S. Miller, Martin Schwidefsky,
	Heiko Carstens, Willem Riede, James E.J. Bottomley,
	Martin K. Petersen, Kai Mäkisara, linux-kernel, linux-ide,
	linux-s390, osst-users, linux-scsi, linux-fsdevel

MTIOCPOS and MTIOCGET are incompatible between 32-bit and 64-bit user
space, and traditionally have been translated in fs/compat_ioctl.c.

To get rid of that translation handler, move a corresponding
implementation into each of the four drivers implementing those commands.

The interesting part of that is now in a new linux/mtio.h header that
wraps the existing uapi/linux/mtio.h header and provides an abstraction
to let drivers handle both cases easily.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/ide/ide-tape.c        | 31 ++++++++++++----
 drivers/s390/char/tape_char.c | 41 ++++++++------------
 drivers/scsi/osst.c           | 34 ++++++++++-------
 drivers/scsi/st.c             | 35 +++++++++++-------
 fs/compat_ioctl.c             | 70 -----------------------------------
 include/linux/mtio.h          | 58 +++++++++++++++++++++++++++++
 6 files changed, 137 insertions(+), 132 deletions(-)
 create mode 100644 include/linux/mtio.h

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 34c1165226a4..137febf3658d 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -19,6 +19,7 @@
 
 #define IDETAPE_VERSION "1.20"
 
+#include <linux/compat.h>
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/string.h>
@@ -1368,7 +1369,7 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count)
  * ide-tape ioctls are supported on both interfaces.
  */
 static long do_idetape_chrdev_ioctl(struct file *file,
-				unsigned int cmd, unsigned long arg)
+				unsigned int cmd, unsigned long arg, bool compat)
 {
 	struct ide_tape_obj *tape = file->private_data;
 	ide_drive_t *drive = tape->drive;
@@ -1407,14 +1408,10 @@ static long do_idetape_chrdev_ioctl(struct file *file,
 		if (tape->drv_write_prot)
 			mtget.mt_gstat |= GMT_WR_PROT(0xffffffff);
 
-		if (copy_to_user(argp, &mtget, sizeof(struct mtget)))
-			return -EFAULT;
-		return 0;
+		return put_user_mtget(argp, &mtget, compat);
 	case MTIOCPOS:
 		mtpos.mt_blkno = position / tape->user_bs_factor - block_offset;
-		if (copy_to_user(argp, &mtpos, sizeof(struct mtpos)))
-			return -EFAULT;
-		return 0;
+		return put_user_mtpos(argp, &mtpos, compat);
 	default:
 		if (tape->chrdev_dir == IDETAPE_DIR_READ)
 			ide_tape_discard_merge_buffer(drive, 1);
@@ -1427,7 +1424,23 @@ static long idetape_chrdev_ioctl(struct file *file,
 {
 	long ret;
 	mutex_lock(&ide_tape_mutex);
-	ret = do_idetape_chrdev_ioctl(file, cmd, arg);
+	ret = do_idetape_chrdev_ioctl(file, cmd, arg, false);
+	mutex_unlock(&ide_tape_mutex);
+	return ret;
+}
+
+static long idetape_chrdev_compat_ioctl(struct file *file,
+				unsigned int cmd, unsigned long arg)
+{
+	long ret;
+
+	if (cmd == MTIOCPOS32)
+		cmd = MTIOCPOS;
+	else if (cmd == MTIOCGET32)
+		cmd = MTIOCGET;
+
+	mutex_lock(&ide_tape_mutex);
+	ret = do_idetape_chrdev_ioctl(file, cmd, arg, true);
 	mutex_unlock(&ide_tape_mutex);
 	return ret;
 }
@@ -1886,6 +1899,8 @@ static const struct file_operations idetape_fops = {
 	.read		= idetape_chrdev_read,
 	.write		= idetape_chrdev_write,
 	.unlocked_ioctl	= idetape_chrdev_ioctl,
+	.compat_ioctl	= IS_ENABLED(CONFIG_COMPAT) ?
+			  idetape_chrdev_compat_ioctl : NULL,
 	.open		= idetape_chrdev_open,
 	.release	= idetape_chrdev_release,
 	.llseek		= noop_llseek,
diff --git a/drivers/s390/char/tape_char.c b/drivers/s390/char/tape_char.c
index fc206c9d1c56..522ca9b836e3 100644
--- a/drivers/s390/char/tape_char.c
+++ b/drivers/s390/char/tape_char.c
@@ -341,14 +341,14 @@ tapechar_release(struct inode *inode, struct file *filp)
  */
 static int
 __tapechar_ioctl(struct tape_device *device,
-		 unsigned int no, unsigned long data)
+		 unsigned int no, void __user *data, bool compat)
 {
 	int rc;
 
 	if (no == MTIOCTOP) {
 		struct mtop op;
 
-		if (copy_from_user(&op, (char __user *) data, sizeof(op)) != 0)
+		if (copy_from_user(&op, data, sizeof(op)) != 0)
 			return -EFAULT;
 		if (op.mt_count < 0)
 			return -EINVAL;
@@ -392,9 +392,7 @@ __tapechar_ioctl(struct tape_device *device,
 		if (rc < 0)
 			return rc;
 		pos.mt_blkno = rc;
-		if (copy_to_user((char __user *) data, &pos, sizeof(pos)) != 0)
-			return -EFAULT;
-		return 0;
+		return put_user_mtpos(data, &pos, compat);
 	}
 	if (no == MTIOCGET) {
 		/* MTIOCGET: query the tape drive status. */
@@ -424,15 +422,12 @@ __tapechar_ioctl(struct tape_device *device,
 			get.mt_blkno = rc;
 		}
 
-		if (copy_to_user((char __user *) data, &get, sizeof(get)) != 0)
-			return -EFAULT;
-
-		return 0;
+		return put_user_mtget(data, &get, compat);
 	}
 	/* Try the discipline ioctl function. */
 	if (device->discipline->ioctl_fn == NULL)
 		return -EINVAL;
-	return device->discipline->ioctl_fn(device, no, data);
+	return device->discipline->ioctl_fn(device, no, (unsigned long)data);
 }
 
 static long
@@ -445,7 +440,7 @@ tapechar_ioctl(struct file *filp, unsigned int no, unsigned long data)
 
 	device = (struct tape_device *) filp->private_data;
 	mutex_lock(&device->mutex);
-	rc = __tapechar_ioctl(device, no, data);
+	rc = __tapechar_ioctl(device, no, (void __user *)data, false);
 	mutex_unlock(&device->mutex);
 	return rc;
 }
@@ -455,23 +450,17 @@ static long
 tapechar_compat_ioctl(struct file *filp, unsigned int no, unsigned long data)
 {
 	struct tape_device *device = filp->private_data;
-	int rval = -ENOIOCTLCMD;
-	unsigned long argp;
+	long rc;
 
-	/* The 'arg' argument of any ioctl function may only be used for
-	 * pointers because of the compat pointer conversion.
-	 * Consider this when adding new ioctls.
-	 */
-	argp = (unsigned long) compat_ptr(data);
-	if (device->discipline->ioctl_fn) {
-		mutex_lock(&device->mutex);
-		rval = device->discipline->ioctl_fn(device, no, argp);
-		mutex_unlock(&device->mutex);
-		if (rval == -EINVAL)
-			rval = -ENOIOCTLCMD;
-	}
+	if (no == MTIOCPOS32)
+		no = MTIOCPOS;
+	else if (no == MTIOCGET32)
+		no = MTIOCGET;
 
-	return rval;
+	mutex_lock(&device->mutex);
+	rc = __tapechar_ioctl(device, no, compat_ptr(data), false);
+	mutex_unlock(&device->mutex);
+	return rc;
 }
 #endif /* CONFIG_COMPAT */
 
diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
index 7a1a1edde35d..842457b9134a 100644
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -33,6 +33,7 @@ static const char * osst_version = "0.99.4";
 
 #include <linux/module.h>
 
+#include <linux/compat.h>
 #include <linux/fs.h>
 #include <linux/kernel.h>
 #include <linux/sched/signal.h>
@@ -4941,7 +4942,7 @@ static int os_scsi_tape_close(struct inode * inode, struct file * filp)
 static long osst_ioctl(struct file * file,
 	 unsigned int cmd_in, unsigned long arg)
 {
-	int		      i, cmd_nr, cmd_type, blk, retval = 0;
+	int		      i, cmd_nr, cmd_type, cmd_size, blk, retval = 0;
 	struct st_modedef   * STm;
 	struct st_partstat  * STps;
 	struct osst_request * SRpnt = NULL;
@@ -4978,6 +4979,7 @@ static long osst_ioctl(struct file * file,
 
 	cmd_type = _IOC_TYPE(cmd_in);
 	cmd_nr   = _IOC_NR(cmd_in);
+	cmd_size = _IOC_SIZE(cmd_in);
 #if DEBUG
 	printk(OSST_DEB_MSG "%s:D: Ioctl %d,%d in %s mode\n", name,
 			    cmd_type, cmd_nr, STp->raw?"raw":"normal");
@@ -5179,7 +5181,8 @@ static long osst_ioctl(struct file * file,
 	if (cmd_type == _IOC_TYPE(MTIOCGET) && cmd_nr == _IOC_NR(MTIOCGET)) {
 		struct mtget mt_status;
 
-		if (_IOC_SIZE(cmd_in) != sizeof(struct mtget)) {
+		if (cmd_size != sizeof(struct mtget) &&
+		    cmd_size != sizeof(struct mtget32)) {
 			 retval = (-EINVAL);
 			 goto out;
 		}
@@ -5229,21 +5232,18 @@ static long osst_ioctl(struct file * file,
 		    STp->drv_buffer != 0)
 			mt_status.mt_gstat |= GMT_IM_REP_EN(0xffffffff);
 
-		i = copy_to_user(p, &mt_status, sizeof(struct mtget));
-		if (i) {
-			retval = (-EFAULT);
-			goto out;
-		}
-
-		STp->recover_erreg = 0;  /* Clear after read */
-		retval = 0;
+		retval = put_user_mtget(p, &mt_status,
+					cmd_size == sizeof(struct mtget32));
+		if (!retval)
+			STp->recover_erreg = 0;  /* Clear after read */
 		goto out;
 	} /* End of MTIOCGET */
 
 	if (cmd_type == _IOC_TYPE(MTIOCPOS) && cmd_nr == _IOC_NR(MTIOCPOS)) {
 		struct mtpos mt_pos;
 
-		if (_IOC_SIZE(cmd_in) != sizeof(struct mtpos)) {
+		if (cmd_size != sizeof(struct mtpos) &&
+		    cmd_size != sizeof(struct mtpos32)) {
 			retval = (-EINVAL);
 			goto out;
 		}
@@ -5256,9 +5256,7 @@ static long osst_ioctl(struct file * file,
 			goto out;
 		}
 		mt_pos.mt_blkno = blk;
-		i = copy_to_user(p, &mt_pos, sizeof(struct mtpos));
-		if (i)
-			retval = -EFAULT;
+		retval = put_user_mtpos(p, &mt_pos, cmd_size == sizeof(struct mtpos32));
 		goto out;
 	}
 	if (SRpnt) osst_release_request(SRpnt);
@@ -5284,6 +5282,14 @@ static long osst_compat_ioctl(struct file * file, unsigned int cmd_in, unsigned
 	struct osst_tape *STp = file->private_data;
 	struct scsi_device *sdev = STp->device;
 	int ret = -ENOIOCTLCMD;
+
+	switch (cmd_in) {
+	case MTIOCTOP:
+	case MTIOCPOS32:
+	case MTIOCGET32:
+		return osst_ioctl(file, cmd_in, (unsigned long)compat_ptr(arg));
+	}
+
 	if (sdev->host->hostt->compat_ioctl) {
 
 		ret = sdev->host->hostt->compat_ioctl(sdev, cmd_in, (void __user *)arg);
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 307df2fa39a3..62244ce53baa 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -21,6 +21,7 @@ static const char *verstr = "20160209";
 
 #include <linux/module.h>
 
+#include <linux/compat.h>
 #include <linux/fs.h>
 #include <linux/kernel.h>
 #include <linux/sched/signal.h>
@@ -3498,7 +3499,7 @@ static int partition_tape(struct scsi_tape *STp, int size)
 /* The ioctl command */
 static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 {
-	int i, cmd_nr, cmd_type, bt;
+	int i, cmd_nr, cmd_type, cmd_size, bt;
 	int retval = 0;
 	unsigned int blk;
 	struct scsi_tape *STp = file->private_data;
@@ -3532,6 +3533,7 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 
 	cmd_type = _IOC_TYPE(cmd_in);
 	cmd_nr = _IOC_NR(cmd_in);
+	cmd_size = _IOC_SIZE(cmd_in);
 
 	if (cmd_type == _IOC_TYPE(MTIOCTOP) && cmd_nr == _IOC_NR(MTIOCTOP)) {
 		struct mtop mtc;
@@ -3741,7 +3743,8 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 	if (cmd_type == _IOC_TYPE(MTIOCGET) && cmd_nr == _IOC_NR(MTIOCGET)) {
 		struct mtget mt_status;
 
-		if (_IOC_SIZE(cmd_in) != sizeof(struct mtget)) {
+		if (cmd_size != sizeof(struct mtget) &&
+		    cmd_size != sizeof(struct mtget32)) {
 			 retval = (-EINVAL);
 			 goto out;
 		}
@@ -3796,19 +3799,16 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 		if (STp->cleaning_req)
 			mt_status.mt_gstat |= GMT_CLN(0xffffffff);
 
-		i = copy_to_user(p, &mt_status, sizeof(struct mtget));
-		if (i) {
-			retval = (-EFAULT);
-			goto out;
-		}
+		retval = put_user_mtget(p, &mt_status,
+					cmd_size == sizeof(struct mtget32));
 
 		STp->recover_reg = 0;		/* Clear after read */
-		retval = 0;
 		goto out;
 	}			/* End of MTIOCGET */
 	if (cmd_type == _IOC_TYPE(MTIOCPOS) && cmd_nr == _IOC_NR(MTIOCPOS)) {
 		struct mtpos mt_pos;
-		if (_IOC_SIZE(cmd_in) != sizeof(struct mtpos)) {
+		if (cmd_size != sizeof(struct mtpos) &&
+		    cmd_size != sizeof(struct mtpos32)) {
 			 retval = (-EINVAL);
 			 goto out;
 		}
@@ -3817,9 +3817,8 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 			goto out;
 		}
 		mt_pos.mt_blkno = blk;
-		i = copy_to_user(p, &mt_pos, sizeof(struct mtpos));
-		if (i)
-			retval = (-EFAULT);
+		retval = put_user_mtpos(p, &mt_pos,
+					cmd_size == sizeof(struct mtpos32));
 		goto out;
 	}
 	mutex_unlock(&STp->lock);
@@ -3853,14 +3852,22 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 }
 
 #ifdef CONFIG_COMPAT
-static long st_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+static long st_compat_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 {
 	struct scsi_tape *STp = file->private_data;
 	struct scsi_device *sdev = STp->device;
 	int ret = -ENOIOCTLCMD;
+
+	switch (cmd_in) {
+	case MTIOCTOP:
+	case MTIOCPOS32:
+	case MTIOCGET32:
+		return st_ioctl(file, cmd_in, (unsigned long)compat_ptr(arg));
+	}
+
 	if (sdev->host->hostt->compat_ioctl) { 
 
-		ret = sdev->host->hostt->compat_ioctl(sdev, cmd, (void __user *)arg);
+		ret = sdev->host->hostt->compat_ioctl(sdev, cmd_in, (void __user *)arg);
 
 	}
 	return ret;
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index bb73d52f02a2..70db940aeea9 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -35,7 +35,6 @@
 #include <linux/ppp_defs.h>
 #include <linux/ppp-ioctl.h>
 #include <linux/if_pppox.h>
-#include <linux/mtio.h>
 #include <linux/tty.h>
 #include <linux/vt_kern.h>
 #include <linux/fb.h>
@@ -265,70 +264,6 @@ static int sg_ioctl_trans(struct file *file, unsigned int cmd,
 	return err;
 }
 
-struct mtget32 {
-	compat_long_t	mt_type;
-	compat_long_t	mt_resid;
-	compat_long_t	mt_dsreg;
-	compat_long_t	mt_gstat;
-	compat_long_t	mt_erreg;
-	compat_daddr_t	mt_fileno;
-	compat_daddr_t	mt_blkno;
-};
-#define MTIOCGET32	_IOR('m', 2, struct mtget32)
-
-struct mtpos32 {
-	compat_long_t	mt_blkno;
-};
-#define MTIOCPOS32	_IOR('m', 3, struct mtpos32)
-
-static int mt_ioctl_trans(struct file *file,
-		unsigned int cmd, void __user *argp)
-{
-	/* NULL initialization to make gcc shut up */
-	struct mtget __user *get = NULL;
-	struct mtget32 __user *umget32;
-	struct mtpos __user *pos = NULL;
-	struct mtpos32 __user *upos32;
-	unsigned long kcmd;
-	void *karg;
-	int err = 0;
-
-	switch(cmd) {
-	case MTIOCPOS32:
-		kcmd = MTIOCPOS;
-		pos = compat_alloc_user_space(sizeof(*pos));
-		karg = pos;
-		break;
-	default:	/* MTIOCGET32 */
-		kcmd = MTIOCGET;
-		get = compat_alloc_user_space(sizeof(*get));
-		karg = get;
-		break;
-	}
-	if (karg == NULL)
-		return -EFAULT;
-	err = do_ioctl(file, kcmd, (unsigned long)karg);
-	if (err)
-		return err;
-	switch (cmd) {
-	case MTIOCPOS32:
-		upos32 = argp;
-		err = convert_in_user(&pos->mt_blkno, &upos32->mt_blkno);
-		break;
-	case MTIOCGET32:
-		umget32 = argp;
-		err = convert_in_user(&get->mt_type, &umget32->mt_type);
-		err |= convert_in_user(&get->mt_resid, &umget32->mt_resid);
-		err |= convert_in_user(&get->mt_dsreg, &umget32->mt_dsreg);
-		err |= convert_in_user(&get->mt_gstat, &umget32->mt_gstat);
-		err |= convert_in_user(&get->mt_erreg, &umget32->mt_erreg);
-		err |= convert_in_user(&get->mt_fileno, &umget32->mt_fileno);
-		err |= convert_in_user(&get->mt_blkno, &umget32->mt_blkno);
-		break;
-	}
-	return err ? -EFAULT: 0;
-}
-
 #endif /* CONFIG_BLOCK */
 
 /* Bluetooth ioctls */
@@ -544,8 +479,6 @@ COMPATIBLE_IOCTL(SCSI_IOCTL_GET_PCI)
  */
 COMPATIBLE_IOCTL(_IOR('p', 20, int[7])) /* RTCGET */
 COMPATIBLE_IOCTL(_IOW('p', 21, int[7])) /* RTCSET */
-/* Little m */
-COMPATIBLE_IOCTL(MTIOCTOP)
 /* Socket level stuff */
 COMPATIBLE_IOCTL(FIOQSIZE)
 #ifdef CONFIG_BLOCK
@@ -657,9 +590,6 @@ static long do_ioctl_trans(unsigned int cmd,
 #ifdef CONFIG_BLOCK
 	case SG_IO:
 		return sg_ioctl_trans(file, cmd, argp);
-	case MTIOCGET32:
-	case MTIOCPOS32:
-		return mt_ioctl_trans(file, cmd, argp);
 #endif
 	/* Serial */
 	case TIOCGSERIAL:
diff --git a/include/linux/mtio.h b/include/linux/mtio.h
new file mode 100644
index 000000000000..02640756a40d
--- /dev/null
+++ b/include/linux/mtio.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_MTIO_COMPAT_H
+#define _LINUX_MTIO_COMPAT_H
+
+#include <uapi/linux/mtio.h>
+#include <linux/uaccess.h>
+
+/*
+ * helper functions for implementing compat ioctls on the four tape
+ * drivers: we define the 32-bit layout of each incompatible strucucture,
+ * plus a wrapper function to copy it to user space in either format.
+ */
+
+struct	mtget32 {
+	s32	mt_type;
+	s32	mt_resid;
+	s32	mt_dsreg;
+	s32	mt_gstat;
+	s32	mt_erreg;
+	s32	mt_fileno;
+	s32	mt_blkno;
+};
+#define	MTIOCGET32	_IOR('m', 2, struct mtget32)
+
+struct	mtpos32 {
+	s32 	mt_blkno;
+};
+#define	MTIOCPOS32	_IOR('m', 3, struct mtpos32)
+
+static inline int put_user_mtget(void __user *u, struct mtget *k, bool compat)
+{
+	struct mtget32 k32 = {
+		.mt_type   = k->mt_type,
+		.mt_resid  = k->mt_resid,
+		.mt_dsreg  = k->mt_dsreg,
+		.mt_gstat  = k->mt_gstat,
+		.mt_fileno = k->mt_fileno,
+		.mt_blkno  = k->mt_blkno,
+	};
+	int ret;
+
+	if (IS_ENABLED(CONFIG_COMPAT) && compat)
+		ret = copy_to_user(u, &k32, sizeof(k32));
+	else
+		ret = copy_to_user(u, k, sizeof(*k));
+
+	return ret ? -EFAULT : 0;
+}
+
+static inline int put_user_mtpos(void __user *u, struct mtpos *k, bool compat)
+{
+	if (IS_ENABLED(CONFIG_COMPAT) && compat)
+		return put_user(k->mt_blkno, (u32 __user *)u);
+	else
+		return put_user(k->mt_blkno, (long __user *)u);
+}
+
+#endif
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 06/11] compat_ioctl: remove /dev/random commands
  2018-09-08 14:28 ` [PATCH 06/11] compat_ioctl: remove /dev/random commands Arnd Bergmann
@ 2018-09-09  4:11   ` Al Viro
  2018-09-11 20:26     ` Arnd Bergmann
  0 siblings, 1 reply; 22+ messages in thread
From: Al Viro @ 2018-09-09  4:11 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Theodore Ts'o, Greg Kroah-Hartman, linux-kernel, linux-fsdevel

On Sat, Sep 08, 2018 at 04:28:12PM +0200, Arnd Bergmann wrote:
> These are all handled by the random driver, so instead of listing
> each ioctl, we can just use the same function to deal with both
> native and compat commands.

Umm...  I don't think it's right -

>  	.unlocked_ioctl = random_ioctl,
> +	.compat_ioctl = random_ioctl,


->compat_ioctl() gets called in
                        error = f.file->f_op->compat_ioctl(f.file, cmd, arg);
so you do *NOT* get compat_ptr() for those - they have to do it on their
own.  It's not hard to provide a proper compat_ioctl() instance for that
one, but this is not it.  What you need in drivers/char/random.c part of
that one is something like

diff --git a/drivers/char/random.c b/drivers/char/random.c
index bf5f99fc36f1..1de75c784cf6 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1954,10 +1954,9 @@ static ssize_t random_write(struct file *file, const char __user *buffer,
 	return (ssize_t)count;
 }
 
-static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
+static long __random_ioctl(struct file *f, unsigned int cmd, int __user *p)
 {
 	int size, ent_count;
-	int __user *p = (int __user *)arg;
 	int retval;
 
 	switch (cmd) {
@@ -2011,6 +2010,18 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
 	}
 }
 
+static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
+{
+	return __random_ioctl(f, cmd, (int __user *)arg);
+}
+
+#ifdef CONFIG_COMPAT
+static long compat_random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
+{
+	return __random_ioctl(f, cmd, compat_ptr(arg));
+}
+#endif
+
 static int random_fasync(int fd, struct file *filp, int on)
 {
 	return fasync_helper(fd, filp, on, &fasync);
@@ -2021,6 +2032,9 @@ const struct file_operations random_fops = {
 	.write = random_write,
 	.poll  = random_poll,
 	.unlocked_ioctl = random_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = compat_random_ioctl,
+#endif
 	.fasync = random_fasync,
 	.llseek = noop_llseek,
 };

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 03/11] compat_ioctl: remove translation for sound ioctls
  2018-09-08 14:28 ` [PATCH 03/11] compat_ioctl: remove translation for sound ioctls Arnd Bergmann
@ 2018-09-09  4:16   ` Al Viro
  2018-09-11 19:35     ` Arnd Bergmann
  0 siblings, 1 reply; 22+ messages in thread
From: Al Viro @ 2018-09-09  4:16 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jeff Dike, Richard Weinberger, Jaroslav Kysela, Takashi Iwai,
	linux-um, linux-kernel, linux-fsdevel, alsa-devel

On Sat, Sep 08, 2018 at 04:28:09PM +0200, Arnd Bergmann wrote:
> The SNDCTL_* and SOUND_* commands are the old OSS user interface.
> 
> I checked all the sound ioctl commands listed in fs/compat_ioctl.c
> to see if we still need the translation handlers. Here is what I
> found:
> 
> - sound/oss/ is (almost) gone from the kernel, this is what actually
>   needed all the translations
> - The ALSA emulation for OSS correctly handles all compat_ioctl
>   commands already.
> - sound/oss/dmasound/ is the last holdout of the original OSS code,
>   this is only used on arch/m68k, which has no 64-bit mode and
>   hence needs no compat handlers
> - arch/um/drivers/hostaudio_kern.c may run in 64-bit mode with
>   32-bit x86 user space underneath it. This rare corner case is
>   the only one that still needs the compat handlers.
> 
> By adding a simple redirect of .compat_ioctl to .unlocked_ioctl in the
> UML driver, we can remove all the COMPATIBLE_IOCTL() annotations without
> a change in functionality. For completeness, I'm adding the same thing
> to the dmasound file, knowing that it makes no difference.

> diff --git a/arch/um/drivers/hostaudio_kern.c b/arch/um/drivers/hostaudio_kern.c
> index 7f9dbdbc4eb7..0278a642a622 100644
> --- a/arch/um/drivers/hostaudio_kern.c
> +++ b/arch/um/drivers/hostaudio_kern.c
> @@ -298,6 +298,7 @@ static const struct file_operations hostaudio_fops = {
>  	.write          = hostaudio_write,
>  	.poll           = hostaudio_poll,
>  	.unlocked_ioctl	= hostaudio_ioctl,
> +	.compat_ioctl	= hostaudio_ioctl,

Umm...  OK, seeing that it's not going to be used on s390...  It's still
not quite right, though, and I'm afraid that places where we have the
same ->unlocked_ioctl and ->compat_ioctl need an audit - there probably
had been other folks who'd stepped into the same.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 11/11] compat_ioctl: move tape handling into drivers
  2018-09-08 14:28 ` [PATCH 11/11] compat_ioctl: move tape handling into drivers Arnd Bergmann
@ 2018-09-09  4:37   ` Al Viro
  2018-09-11 15:36     ` Arnd Bergmann
  0 siblings, 1 reply; 22+ messages in thread
From: Al Viro @ 2018-09-09  4:37 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David S. Miller, Martin Schwidefsky, Heiko Carstens,
	Willem Riede, James E.J. Bottomley, Martin K. Petersen,
	Kai Mäkisara, linux-kernel, linux-ide, linux-s390,
	osst-users, linux-scsi, linux-fsdevel

On Sat, Sep 08, 2018 at 04:28:17PM +0200, Arnd Bergmann wrote:
> MTIOCPOS and MTIOCGET are incompatible between 32-bit and 64-bit user
> space, and traditionally have been translated in fs/compat_ioctl.c.
> 
> To get rid of that translation handler, move a corresponding
> implementation into each of the four drivers implementing those commands.
> 
> The interesting part of that is now in a new linux/mtio.h header that
> wraps the existing uapi/linux/mtio.h header and provides an abstraction
> to let drivers handle both cases easily.

Ugh...  Frankly, this bool compat passed all way down looks wrong.
I can live with that; the question is whether block folks will be
OK with that thing...

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 11/11] compat_ioctl: move tape handling into drivers
  2018-09-09  4:37   ` Al Viro
@ 2018-09-11 15:36     ` Arnd Bergmann
  2018-09-11 20:13       ` Arnd Bergmann
  0 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-11 15:36 UTC (permalink / raw)
  To: Al Viro
  Cc: David Miller, Martin Schwidefsky, Heiko Carstens, osst,
	James E.J. Bottomley, Martin K. Petersen, Kai.Makisara,
	Linux Kernel Mailing List, IDE-ML, linux-s390, osst-users,
	linux-scsi, Linux FS-devel Mailing List

On Sun, Sep 9, 2018 at 6:38 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Sat, Sep 08, 2018 at 04:28:17PM +0200, Arnd Bergmann wrote:
> > MTIOCPOS and MTIOCGET are incompatible between 32-bit and 64-bit user
> > space, and traditionally have been translated in fs/compat_ioctl.c.
> >
> > To get rid of that translation handler, move a corresponding
> > implementation into each of the four drivers implementing those commands.
> >
> > The interesting part of that is now in a new linux/mtio.h header that
> > wraps the existing uapi/linux/mtio.h header and provides an abstraction
> > to let drivers handle both cases easily.
>
> Ugh...  Frankly, this bool compat passed all way down looks wrong.
> I can live with that; the question is whether block folks will be
> OK with that thing...

I have tried to come up with an alternative, but couldn't really find anything
that is less ugly. Since nobody else complained, I'll resend this version
along with the other patches.

       Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 03/11] compat_ioctl: remove translation for sound ioctls
  2018-09-09  4:16   ` Al Viro
@ 2018-09-11 19:35     ` Arnd Bergmann
  0 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-11 19:35 UTC (permalink / raw)
  To: Al Viro
  Cc: Jeff Dike, Richard Weinberger, Jaroslav Kysela, Takashi Iwai,
	linux-um, Linux Kernel Mailing List, Linux FS-devel Mailing List,
	alsa-devel

On Sun, Sep 9, 2018 at 6:17 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Sat, Sep 08, 2018 at 04:28:09PM +0200, Arnd Bergmann wrote:
> > The SNDCTL_* and SOUND_* commands are the old OSS user interface.
> >
> > I checked all the sound ioctl commands listed in fs/compat_ioctl.c
> > to see if we still need the translation handlers. Here is what I
> > found:
> >
> > - sound/oss/ is (almost) gone from the kernel, this is what actually
> >   needed all the translations
> > - The ALSA emulation for OSS correctly handles all compat_ioctl
> >   commands already.
> > - sound/oss/dmasound/ is the last holdout of the original OSS code,
> >   this is only used on arch/m68k, which has no 64-bit mode and
> >   hence needs no compat handlers
> > - arch/um/drivers/hostaudio_kern.c may run in 64-bit mode with
> >   32-bit x86 user space underneath it. This rare corner case is
> >   the only one that still needs the compat handlers.
> >
> > By adding a simple redirect of .compat_ioctl to .unlocked_ioctl in the
> > UML driver, we can remove all the COMPATIBLE_IOCTL() annotations without
> > a change in functionality. For completeness, I'm adding the same thing
> > to the dmasound file, knowing that it makes no difference.
>
> > diff --git a/arch/um/drivers/hostaudio_kern.c b/arch/um/drivers/hostaudio_kern.c
> > index 7f9dbdbc4eb7..0278a642a622 100644
> > --- a/arch/um/drivers/hostaudio_kern.c
> > +++ b/arch/um/drivers/hostaudio_kern.c
> > @@ -298,6 +298,7 @@ static const struct file_operations hostaudio_fops = {
> >       .write          = hostaudio_write,
> >       .poll           = hostaudio_poll,
> >       .unlocked_ioctl = hostaudio_ioctl,
> > +     .compat_ioctl   = hostaudio_ioctl,
>
> Umm...  OK, seeing that it's not going to be used on s390...  It's still
> not quite right, though, and I'm afraid that places where we have the
> same ->unlocked_ioctl and ->compat_ioctl need an audit - there probably
> had been other folks who'd stepped into the same.

It turns out that it's actually much more common to have the same
pointer for ioctl handlers that thake pointer arguments than to have
the wrapper. I found 16 instances of trivial wrappers to do compat_ptr()
for file_operations, 4 instances that have a wrapper but skip the
compat_ptr() and around 40 (depending on how you count) that use
the same pointer for both when they only use pointers and should
go through compat_ptr(). This includes a couple that don't use
the argument at all, so they are fine either way.

I've created patches to change all of the above to a new
generic_compat_ioctl_ptrarg() helper I added.

loop_control_ioctl(), kcov_ioctl(), proc_bus_pci_ioctl(), and nbd_ioctl() seem
to be ones that are correct because,  the argument is always an integer.
fs3270_ioctl() has a is_compat_task() check to deal with the problem.

inotify_ioctl(),  vsoc_ioctl(), and usblp_ioctl() are somethat uses both integer
and pointer arguments and may need special handling for s390.

I did not touch those so far.

       Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 11/11] compat_ioctl: move tape handling into drivers
  2018-09-11 15:36     ` Arnd Bergmann
@ 2018-09-11 20:13       ` Arnd Bergmann
  0 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-11 20:13 UTC (permalink / raw)
  To: Al Viro
  Cc: David Miller, Martin Schwidefsky, Heiko Carstens, osst,
	James E.J. Bottomley, Martin K. Petersen, Kai.Makisara,
	Linux Kernel Mailing List, IDE-ML, linux-s390, osst-users,
	linux-scsi, Linux FS-devel Mailing List

On Tue, Sep 11, 2018 at 5:36 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sun, Sep 9, 2018 at 6:38 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Sat, Sep 08, 2018 at 04:28:17PM +0200, Arnd Bergmann wrote:
> > > MTIOCPOS and MTIOCGET are incompatible between 32-bit and 64-bit user
> > > space, and traditionally have been translated in fs/compat_ioctl.c.
> > >
> > > To get rid of that translation handler, move a corresponding
> > > implementation into each of the four drivers implementing those commands.
> > >
> > > The interesting part of that is now in a new linux/mtio.h header that
> > > wraps the existing uapi/linux/mtio.h header and provides an abstraction
> > > to let drivers handle both cases easily.
> >
> > Ugh...  Frankly, this bool compat passed all way down looks wrong.
> > I can live with that; the question is whether block folks will be
> > OK with that thing...
>
> I have tried to come up with an alternative, but couldn't really find anything
> that is less ugly. Since nobody else complained, I'll resend this version
> along with the other patches.

Actually, there was one idea that Deepa mentioned for another subsystem
with a similar issue: instead of passing down the fact that we come from
a compat syscall through multiple function calls, the lowest ones
(put_user_mtpos, put_user_mtget) could call in_compat_syscall().

Would you prefer that?

      Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 06/11] compat_ioctl: remove /dev/random commands
  2018-09-09  4:11   ` Al Viro
@ 2018-09-11 20:26     ` Arnd Bergmann
  2018-09-12  5:28       ` Martin Schwidefsky
  0 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-11 20:26 UTC (permalink / raw)
  To: Al Viro
  Cc: Theodore Ts'o, gregkh, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, linux-s390, Martin Schwidefsky,
	Heiko Carstens

On Sun, Sep 9, 2018 at 6:12 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Sat, Sep 08, 2018 at 04:28:12PM +0200, Arnd Bergmann wrote:
> > These are all handled by the random driver, so instead of listing
> > each ioctl, we can just use the same function to deal with both
> > native and compat commands.
>
> Umm...  I don't think it's right -
>
> >       .unlocked_ioctl = random_ioctl,
> > +     .compat_ioctl = random_ioctl,
>
>
> ->compat_ioctl() gets called in
>                         error = f.file->f_op->compat_ioctl(f.file, cmd, arg);
> so you do *NOT* get compat_ptr() for those - they have to do it on their
> own.  It's not hard to provide a proper compat_ioctl() instance for that
> one, but this is not it.  What you need in drivers/char/random.c part of
> that one is something like

Looping in some s390 folks.

As you suggested in another reply, I had a look at what other drivers
do the same thing and have only pointer arguments. I created a
patch to move them all over to using a new helper function that
adds the compat_ptr(), and arrived at

 drivers/android/binder.c                    | 2 +-
 drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +-
 drivers/dma-buf/dma-buf.c                   | 4 +---
 drivers/dma-buf/sw_sync.c                   | 2 +-
 drivers/dma-buf/sync_file.c                 | 2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c    | 2 +-
 drivers/hid/hidraw.c                        | 4 +---
 drivers/iio/industrialio-core.c             | 2 +-
 drivers/infiniband/core/uverbs_main.c       | 4 ++--
 drivers/media/rc/lirc_dev.c                 | 4 +---
 drivers/mfd/cros_ec_dev.c                   | 4 +---
 drivers/misc/vmw_vmci/vmci_host.c           | 2 +-
 drivers/nvdimm/bus.c                        | 4 ++--
 drivers/nvme/host/core.c                    | 6 +++---
 drivers/pci/switch/switchtec.c              | 2 +-
 drivers/platform/x86/wmi.c                  | 2 +-
 drivers/rpmsg/rpmsg_char.c                  | 4 ++--
 drivers/s390/char/sclp_ctl.c                | 8 ++------
 drivers/s390/char/vmcp.c                    | 2 ++----
 drivers/s390/cio/chsc_sch.c                 | 8 ++------
 drivers/sbus/char/display7seg.c             | 2 +-
 drivers/sbus/char/envctrl.c                 | 4 +---
 drivers/scsi/3w-xxxx.c                      | 4 +---
 drivers/scsi/cxlflash/main.c                | 2 +-
 drivers/scsi/esas2r/esas2r_main.c           | 2 +-
 drivers/scsi/pmcraid.c                      | 4 +---
 drivers/staging/android/ion/ion.c           | 4 +---
 drivers/staging/vme/devices/vme_user.c      | 2 +-
 drivers/tee/tee_core.c                      | 2 +-
 drivers/usb/class/cdc-wdm.c                 | 2 +-
 drivers/usb/class/usbtmc.c                  | 4 +---
 drivers/video/fbdev/ps3fb.c                 | 2 +-
 drivers/video/fbdev/sis/sis_main.c          | 4 +---
 drivers/virt/fsl_hypervisor.c               | 2 +-
 fs/btrfs/super.c                            | 2 +-
 fs/ceph/dir.c                               | 2 +-
 fs/ceph/file.c                              | 2 +-
 fs/fuse/dev.c                               | 2 +-
 fs/notify/fanotify/fanotify_user.c          | 2 +-
 fs/userfaultfd.c                            | 2 +-
 net/rfkill/core.c                           | 2 +-
 41 files changed, 48 insertions(+), 76 deletions(-)

Out of those, there are only a few that may get used on s390,
in particular at most infiniband/uverbs, nvme, nvdimm,
btrfs, ceph, fuse, fanotify and userfaultfd.
[Note: there are three s390 drivers in the list, which use
a different method: they check in_compat_syscall() from
a shared handler to decide whether to do compat_ptr().

According to my memory from when I last worked on this,
the compat_ptr() is mainly a safeguard for legacy binaries
that got created with ancient C compilers (or compilers for
something other than C)  and might leave the high bit set
in a pointer, but modern C compilers (gcc-3+) won't ever
do that.

You are probably right about /dev/random, which could be
used in lots of weird code, but I wonder to what degree we
need to worry about it for the rest.

       Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 06/11] compat_ioctl: remove /dev/random commands
  2018-09-11 20:26     ` Arnd Bergmann
@ 2018-09-12  5:28       ` Martin Schwidefsky
  2018-09-12 14:02         ` Arnd Bergmann
  0 siblings, 1 reply; 22+ messages in thread
From: Martin Schwidefsky @ 2018-09-12  5:28 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Al Viro, Theodore Ts'o, gregkh, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, linux-s390, Heiko Carstens

On Tue, 11 Sep 2018 22:26:54 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Sun, Sep 9, 2018 at 6:12 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Sat, Sep 08, 2018 at 04:28:12PM +0200, Arnd Bergmann wrote:  
> > > These are all handled by the random driver, so instead of listing
> > > each ioctl, we can just use the same function to deal with both
> > > native and compat commands.  
> >
> > Umm...  I don't think it's right -
> >  
> > >       .unlocked_ioctl = random_ioctl,
> > > +     .compat_ioctl = random_ioctl,  
> >
> >  
> > ->compat_ioctl() gets called in  
> >                         error = f.file->f_op->compat_ioctl(f.file, cmd, arg);
> > so you do *NOT* get compat_ptr() for those - they have to do it on their
> > own.  It's not hard to provide a proper compat_ioctl() instance for that
> > one, but this is not it.  What you need in drivers/char/random.c part of
> > that one is something like  
> 
> Looping in some s390 folks.
> 
> As you suggested in another reply, I had a look at what other drivers
> do the same thing and have only pointer arguments. I created a
> patch to move them all over to using a new helper function that
> adds the compat_ptr(), and arrived at
> 
>  drivers/android/binder.c                    | 2 +-
>  drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +-
>  drivers/dma-buf/dma-buf.c                   | 4 +---
>  drivers/dma-buf/sw_sync.c                   | 2 +-
>  drivers/dma-buf/sync_file.c                 | 2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c    | 2 +-
>  drivers/hid/hidraw.c                        | 4 +---
>  drivers/iio/industrialio-core.c             | 2 +-
>  drivers/infiniband/core/uverbs_main.c       | 4 ++--
>  drivers/media/rc/lirc_dev.c                 | 4 +---
>  drivers/mfd/cros_ec_dev.c                   | 4 +---
>  drivers/misc/vmw_vmci/vmci_host.c           | 2 +-
>  drivers/nvdimm/bus.c                        | 4 ++--
>  drivers/nvme/host/core.c                    | 6 +++---
>  drivers/pci/switch/switchtec.c              | 2 +-
>  drivers/platform/x86/wmi.c                  | 2 +-
>  drivers/rpmsg/rpmsg_char.c                  | 4 ++--
>  drivers/s390/char/sclp_ctl.c                | 8 ++------
>  drivers/s390/char/vmcp.c                    | 2 ++----
>  drivers/s390/cio/chsc_sch.c                 | 8 ++------
>  drivers/sbus/char/display7seg.c             | 2 +-
>  drivers/sbus/char/envctrl.c                 | 4 +---
>  drivers/scsi/3w-xxxx.c                      | 4 +---
>  drivers/scsi/cxlflash/main.c                | 2 +-
>  drivers/scsi/esas2r/esas2r_main.c           | 2 +-
>  drivers/scsi/pmcraid.c                      | 4 +---
>  drivers/staging/android/ion/ion.c           | 4 +---
>  drivers/staging/vme/devices/vme_user.c      | 2 +-
>  drivers/tee/tee_core.c                      | 2 +-
>  drivers/usb/class/cdc-wdm.c                 | 2 +-
>  drivers/usb/class/usbtmc.c                  | 4 +---
>  drivers/video/fbdev/ps3fb.c                 | 2 +-
>  drivers/video/fbdev/sis/sis_main.c          | 4 +---
>  drivers/virt/fsl_hypervisor.c               | 2 +-
>  fs/btrfs/super.c                            | 2 +-
>  fs/ceph/dir.c                               | 2 +-
>  fs/ceph/file.c                              | 2 +-
>  fs/fuse/dev.c                               | 2 +-
>  fs/notify/fanotify/fanotify_user.c          | 2 +-
>  fs/userfaultfd.c                            | 2 +-
>  net/rfkill/core.c                           | 2 +-
>  41 files changed, 48 insertions(+), 76 deletions(-)
> 
> Out of those, there are only a few that may get used on s390,
> in particular at most infiniband/uverbs, nvme, nvdimm,
> btrfs, ceph, fuse, fanotify and userfaultfd.
> [Note: there are three s390 drivers in the list, which use
> a different method: they check in_compat_syscall() from
> a shared handler to decide whether to do compat_ptr().

Using in_compat_syscall() seems to be a good solution, no?

> According to my memory from when I last worked on this,
> the compat_ptr() is mainly a safeguard for legacy binaries
> that got created with ancient C compilers (or compilers for
> something other than C)  and might leave the high bit set
> in a pointer, but modern C compilers (gcc-3+) won't ever
> do that.

And compat_ptr clears the upper 32-bit of the register. If
the register is loaded to e.g. "lr" or "l" there will be
junk in the 4 upper bytes.

> You are probably right about /dev/random, which could be
> used in lots of weird code, but I wonder to what degree we
> need to worry about it for the rest.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 06/11] compat_ioctl: remove /dev/random commands
  2018-09-12  5:28       ` Martin Schwidefsky
@ 2018-09-12 14:02         ` Arnd Bergmann
  2018-09-13  6:42           ` Martin Schwidefsky
  0 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-12 14:02 UTC (permalink / raw)
  To: Martin Schwidefsky
  Cc: Al Viro, Theodore Ts'o, gregkh, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, linux-s390, Heiko Carstens

On Wed, Sep 12, 2018 at 7:29 AM Martin Schwidefsky
<schwidefsky@de.ibm.com> wrote:
> On Tue, 11 Sep 2018 22:26:54 +0200 Arnd Bergmann <arnd@arndb.de> wrote:
> > On Sun, Sep 9, 2018 at 6:12 AM Al Viro <viro@zeniv.linux.org.uk> wrote:

> > Out of those, there are only a few that may get used on s390,
> > in particular at most infiniband/uverbs, nvme, nvdimm,
> > btrfs, ceph, fuse, fanotify and userfaultfd.
> > [Note: there are three s390 drivers in the list, which use
> > a different method: they check in_compat_syscall() from
> > a shared handler to decide whether to do compat_ptr().
>
> Using in_compat_syscall() seems to be a good solution, no?

It works fine for you, but wouldn't work on architecture-independent
code, since 32-bit architectures generally don't provide
a compat_ptr() implementation. This could of course
be changed easily, but after this change it, your drivers
work just as well with a couple few lines, and more consistent
with other drivers:

--- a/drivers/s390/char/sclp_ctl.c
+++ b/drivers/s390/char/sclp_ctl.c
@@ -93,12 +93,8 @@ static int sclp_ctl_ioctl_sccb(void __user *user_area)
 static long sclp_ctl_ioctl(struct file *filp, unsigned int cmd,
                           unsigned long arg)
 {
-       void __user *argp;
+       void __user *argp = (void __user *)arg;

-       if (is_compat_task())
-               argp = compat_ptr(arg);
-       else
-               argp = (void __user *) arg;
        switch (cmd) {
        case SCLP_CTL_SCCB:
                return sclp_ctl_ioctl_sccb(argp);
@@ -114,7 +110,7 @@ static const struct file_operations sclp_ctl_fops = {
        .owner = THIS_MODULE,
        .open = nonseekable_open,
        .unlocked_ioctl = sclp_ctl_ioctl,
-       .compat_ioctl = sclp_ctl_ioctl,
+       .compat_ioctl = generic_compat_ioctl_ptrarg,
        .llseek = no_llseek,
 };

This should probably be separate from the change to using compat_ptr()
in all other drivers, and I could easily drop this change if you prefer,
it is meant only as a cosmetic change.

> > According to my memory from when I last worked on this,
> > the compat_ptr() is mainly a safeguard for legacy binaries
> > that got created with ancient C compilers (or compilers for
> > something other than C)  and might leave the high bit set
> > in a pointer, but modern C compilers (gcc-3+) won't ever
> > do that.
>
> And compat_ptr clears the upper 32-bit of the register. If
> the register is loaded to e.g. "lr" or "l" there will be
> junk in the 4 upper bytes.

I don't think we hit that problem anywhere: in the ioctl
argument we pass an 'unsigned long' that has already
been zero-extended by the compat_sys_ioctl() wrapper,
while any other usage would get extended by the compiler
when casting from compat_uptr_t to a 64-bit type.
This would be different if you had a function call with the
wrong prototype, i.e. calling a function declared as taking
an compat_uptr_t, but defining it as taking a void __user*.
(I suppose that is undefined behavior).

Unless I'm missing something, compat_ptr() should
always just clear bit 31. What I'd like to confirm is whether
you have encountered any code that actually passes
a pointer with that bit set by a user application in the
past 15 years. As Al said, it's probably best to just always
apply the compat_ptr() conversion in each case that s390
needs it even for drivers that don't run on s390, but I'd also
like to understand how much it matters in practice.
(A separate question would be how long we expect to need
32 bit compat mode on arch/s390 at all, but for the moment
I assume this is not up for debate at all).

       Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 06/11] compat_ioctl: remove /dev/random commands
  2018-09-12 14:02         ` Arnd Bergmann
@ 2018-09-13  6:42           ` Martin Schwidefsky
  2018-09-13  8:13             ` Arnd Bergmann
  0 siblings, 1 reply; 22+ messages in thread
From: Martin Schwidefsky @ 2018-09-13  6:42 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Al Viro, Theodore Ts'o, gregkh, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, linux-s390, Heiko Carstens

On Wed, 12 Sep 2018 16:02:40 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Wed, Sep 12, 2018 at 7:29 AM Martin Schwidefsky
> <schwidefsky@de.ibm.com> wrote:
> > On Tue, 11 Sep 2018 22:26:54 +0200 Arnd Bergmann <arnd@arndb.de> wrote:  
> > > On Sun, Sep 9, 2018 at 6:12 AM Al Viro <viro@zeniv.linux.org.uk> wrote:  
> 
> > > Out of those, there are only a few that may get used on s390,
> > > in particular at most infiniband/uverbs, nvme, nvdimm,
> > > btrfs, ceph, fuse, fanotify and userfaultfd.
> > > [Note: there are three s390 drivers in the list, which use
> > > a different method: they check in_compat_syscall() from
> > > a shared handler to decide whether to do compat_ptr().  
> >
> > Using in_compat_syscall() seems to be a good solution, no?  
> 
> It works fine for you, but wouldn't work on architecture-independent
> code, since 32-bit architectures generally don't provide
> a compat_ptr() implementation. This could of course
> be changed easily, but after this change it, your drivers
> work just as well with a couple few lines, and more consistent
> with other drivers:
> 
> --- a/drivers/s390/char/sclp_ctl.c
> +++ b/drivers/s390/char/sclp_ctl.c
> @@ -93,12 +93,8 @@ static int sclp_ctl_ioctl_sccb(void __user *user_area)
>  static long sclp_ctl_ioctl(struct file *filp, unsigned int cmd,
>                            unsigned long arg)
>  {
> -       void __user *argp;
> +       void __user *argp = (void __user *)arg;
> 
> -       if (is_compat_task())
> -               argp = compat_ptr(arg);
> -       else
> -               argp = (void __user *) arg;
>         switch (cmd) {
>         case SCLP_CTL_SCCB:
>                 return sclp_ctl_ioctl_sccb(argp);
> @@ -114,7 +110,7 @@ static const struct file_operations sclp_ctl_fops = {
>         .owner = THIS_MODULE,
>         .open = nonseekable_open,
>         .unlocked_ioctl = sclp_ctl_ioctl,
> -       .compat_ioctl = sclp_ctl_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>         .llseek = no_llseek,
>  };
> 
> This should probably be separate from the change to using compat_ptr()
> in all other drivers, and I could easily drop this change if you prefer,
> it is meant only as a cosmetic change.

So generic_compat_ioctl_ptrarg will to the compat_ptr thing on the
"unsigned int cmd" argument? Should work just fine.


> > > According to my memory from when I last worked on this,
> > > the compat_ptr() is mainly a safeguard for legacy binaries
> > > that got created with ancient C compilers (or compilers for
> > > something other than C)  and might leave the high bit set
> > > in a pointer, but modern C compilers (gcc-3+) won't ever
> > > do that.  
> >
> > And compat_ptr clears the upper 32-bit of the register. If
> > the register is loaded to e.g. "lr" or "l" there will be
> > junk in the 4 upper bytes.  
> 
> I don't think we hit that problem anywhere: in the ioctl
> argument we pass an 'unsigned long' that has already
> been zero-extended by the compat_sys_ioctl() wrapper,
> while any other usage would get extended by the compiler
> when casting from compat_uptr_t to a 64-bit type.
> This would be different if you had a function call with the
> wrong prototype, i.e. calling a function declared as taking
> an compat_uptr_t, but defining it as taking a void __user*.
> (I suppose that is undefined behavior).

That is true. For the ioctls we have a compat "unsigned int"
or "unsigned long" and the system call wrapper must have cleared
the upper half already. There are a few places where we copy
a data structure from user space, then read a 32-bit pointer
from the structure. These get the compat_ptr treatment as well.
All of those structure definitions should use compat_uptr_t
though, the compiler has to do the zero extension at the time
the 32-bit value is cast to a pointer.

> Unless I'm missing something, compat_ptr() should
> always just clear bit 31. What I'd like to confirm is whether
> you have encountered any code that actually passes
> a pointer with that bit set by a user application in the
> past 15 years. As Al said, it's probably best to just always
> apply the compat_ptr() conversion in each case that s390
> needs it even for drivers that don't run on s390, but I'd also
> like to understand how much it matters in practice.
> (A separate question would be how long we expect to need
> 32 bit compat mode on arch/s390 at all, but for the moment
> I assume this is not up for debate at all).

I don't know if that is worth the risk, yes it should work if
compat_ptr just removes bit 31 and leaves the other bits alone.
But if you have to clear one bit, you can as well remove all
the other bits as well.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 06/11] compat_ioctl: remove /dev/random commands
  2018-09-13  6:42           ` Martin Schwidefsky
@ 2018-09-13  8:13             ` Arnd Bergmann
  0 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2018-09-13  8:13 UTC (permalink / raw)
  To: Martin Schwidefsky
  Cc: Al Viro, Theodore Ts'o, gregkh, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, linux-s390, Heiko Carstens

On Thu, Sep 13, 2018 at 8:42 AM Martin Schwidefsky
<schwidefsky@de.ibm.com> wrote:
>
> On Wed, 12 Sep 2018 16:02:40 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
>
> > On Wed, Sep 12, 2018 at 7:29 AM Martin Schwidefsky
> > <schwidefsky@de.ibm.com> wrote:
> > > On Tue, 11 Sep 2018 22:26:54 +0200 Arnd Bergmann <arnd@arndb.de> wrote:

> >
> > This should probably be separate from the change to using compat_ptr()
> > in all other drivers, and I could easily drop this change if you prefer,
> > it is meant only as a cosmetic change.
>
> So generic_compat_ioctl_ptrarg will to the compat_ptr thing on the
> "unsigned int cmd" argument? Should work just fine.

It will do it on the "unsigned long arg" argument, I assume that's
what you meant. The "cmd" argument is correctly zero-extended
by the COMPAT_SYSCALL_DEFINE() wrapper on architectures
that need that (IIRC s390 is in that category).

> > I don't think we hit that problem anywhere: in the ioctl
> > argument we pass an 'unsigned long' that has already
> > been zero-extended by the compat_sys_ioctl() wrapper,
> > while any other usage would get extended by the compiler
> > when casting from compat_uptr_t to a 64-bit type.
> > This would be different if you had a function call with the
> > wrong prototype, i.e. calling a function declared as taking
> > an compat_uptr_t, but defining it as taking a void __user*.
> > (I suppose that is undefined behavior).
>
> That is true. For the ioctls we have a compat "unsigned int"
> or "unsigned long" and the system call wrapper must have cleared
> the upper half already. There are a few places where we copy
> a data structure from user space, then read a 32-bit pointer
> from the structure. These get the compat_ptr treatment as well.
> All of those structure definitions should use compat_uptr_t
> though, the compiler has to do the zero extension at the time
> the 32-bit value is cast to a pointer.

There is actually one more case: A number of the newer
interfaces that have ioctl structures with indirect pointers
encoded as __u64, so the layout becomes
and we don't normally need a conversion handler.

An example of this would be the sys_rseq() system call
that passes a relatively complex structure in place
of a pointer:

struct rseq {
        ...
        union {
                __u64 ptr64;
#ifdef __LP64__
                __u64 ptr;
#else
                struct {
#if (defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) ||
defined(__BIG_ENDIAN)
                        __u32 padding;          /* Initialized to zero. */
                        __u32 ptr32;
#else /* LITTLE */
                        __u32 ptr32;
                        __u32 padding;          /* Initialized to zero. */
#endif /* ENDIAN */
                } ptr;
#endif
        } rseq_cs;
       __u32 flags;
};

We require user space to initialize the __padding field to
zero and then use the ptr64 field in the kernel as a pointer:

        u64 ptr;
        u32 __user *usig;
        copy_from_user(&ptr, &t->rseq->rseq_cs.ptr64, sizeof(ptr));
        urseq_cs = (struct rseq_cs __user *)(unsigned long)ptr;

but we don't ever clear bit 31 here. A similar pattern is used
in many device drivers (I could not find any that would apply to
s390 though). In theory, 32 bit user space might pass a pointer
with the high bit set in the ptr32 field, and that gets misinterpreted
by the kernel (resulting in -EFAULT).

It would be interesting to know whether there could be user space
that gets compiled from portable source code with a normal
C compiler but produces that high bit set, as opposed to someone
intentially settting the bit just to trigger the bug.

       Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2018-09-13 13:22 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-08 14:28 [PATCH 01/11] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann
2018-09-08 14:28 ` [PATCH 02/11] compat_ioctl: remove HIDIO translation Arnd Bergmann
2018-09-08 14:28 ` [PATCH 03/11] compat_ioctl: remove translation for sound ioctls Arnd Bergmann
2018-09-09  4:16   ` Al Viro
2018-09-11 19:35     ` Arnd Bergmann
2018-09-08 14:28 ` [PATCH 04/11] compat_ioctl: remove isdn ioctl translation Arnd Bergmann
2018-09-08 14:28 ` [PATCH 05/11] compat_ioctl: remove IGNORE_IOCTL() Arnd Bergmann
2018-09-08 14:28 ` [PATCH 06/11] compat_ioctl: remove /dev/random commands Arnd Bergmann
2018-09-09  4:11   ` Al Viro
2018-09-11 20:26     ` Arnd Bergmann
2018-09-12  5:28       ` Martin Schwidefsky
2018-09-12 14:02         ` Arnd Bergmann
2018-09-13  6:42           ` Martin Schwidefsky
2018-09-13  8:13             ` Arnd Bergmann
2018-09-08 14:28 ` [PATCH 07/11] compat_ioctl: remove joystick ioctl translation Arnd Bergmann
2018-09-08 14:28 ` [PATCH 08/11] compat_ioctl: remove PCI " Arnd Bergmann
2018-09-08 14:28 ` [PATCH 09/11] compat_ioctl: remove /dev/raw " Arnd Bergmann
2018-09-08 14:28 ` [PATCH 10/11] compat_ioctl: remove last RAID handling code Arnd Bergmann
2018-09-08 14:28 ` [PATCH 11/11] compat_ioctl: move tape handling into drivers Arnd Bergmann
2018-09-09  4:37   ` Al Viro
2018-09-11 15:36     ` Arnd Bergmann
2018-09-11 20:13       ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).