At this commit,
Cc: Linus Torvalds <[email protected]>
commit 07d106d0a33d6063d2061305903deb02489eba20
Author: Linus Torvalds <[email protected]>
Date: Thu Jan 5 15:40:12 2012 -0800
vfs: fix up ENOIOCTLCMD error handling
We're doing some odd things there, which already messes up various users
(see the net/socket.c code that this removes), and it was going to add
yet more crud to the block layer because of the incorrect error code
translation.
ENOIOCTLCMD is not an error return that should be returned to user mode
from the "ioctl()" system call, but it should *not* be translated as
EINVAL ("Invalid argument"). It should be translated as ENOTTY
("Inappropriate ioctl for device").
That EINVAL confusion has apparently so permeated some code that the
block layer actually checks for it, which is sad. We continue to do so
for now, but add a big comment about how wrong that is, and we should
remove it entirely eventually. In the meantime, this tries to keep the
changes localized to just the EINVAL -> ENOTTY fix, and removing code
that makes it harder to do the right thing.
Signed-off-by: Linus Torvalds <[email protected]>
Linus pointed out that ENOIOCTLCMD should be translated as ENOTTY to user
mode, and we should fix up the broken drivers then.
Wanlong Gao (5):
drivers:tty:fix up ENOIOCTLCMD error handling
net:atm:fix up ENOIOCTLCMD error handling
media:dvb:fix up ENOIOCTLCMD error handling
video:omap3isp:fix up ENOIOCTLCMD error handling
s390:block:fix up ENOIOCTLCMD error handling
drivers/media/dvb/dvb-core/dvbdev.c | 2 +-
drivers/media/video/omap3isp/ispvideo.c | 10 +++++-----
drivers/s390/block/dasd_ioctl.c | 2 +-
drivers/tty/tty_io.c | 2 +-
net/atm/resources.c | 2 +-
5 files changed, 9 insertions(+), 9 deletions(-)
--
1.7.12
At commit 07d106d0, Linus pointed out that ENOIOCTLCMD should be
translated as ENOTTY to user mode.
Cc: "David S. Miller" <[email protected]>
Cc: [email protected]
Signed-off-by: Wanlong Gao <[email protected]>
---
net/atm/resources.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/atm/resources.c b/net/atm/resources.c
index 23f45ce..0447d5d 100644
--- a/net/atm/resources.c
+++ b/net/atm/resources.c
@@ -432,7 +432,7 @@ int atm_dev_ioctl(unsigned int cmd, void __user *arg, int compat)
size = dev->ops->ioctl(dev, cmd, buf);
}
if (size < 0) {
- error = (size == -ENOIOCTLCMD ? -EINVAL : size);
+ error = (size == -ENOIOCTLCMD ? -ENOTTY : size);
goto done;
}
}
--
1.7.12
At commit 07d106d0, Linus pointed out that ENOIOCTLCMD should be
translated as ENOTTY to user mode.
Cc: Laurent Pinchart <[email protected]>
Cc: Mauro Carvalho Chehab <[email protected]>
Cc: [email protected]
Signed-off-by: Wanlong Gao <[email protected]>
---
drivers/media/video/omap3isp/ispvideo.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/video/omap3isp/ispvideo.c b/drivers/media/video/omap3isp/ispvideo.c
index b37379d..2dd982e 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -337,7 +337,7 @@ __isp_video_get_format(struct isp_video *video, struct v4l2_format *format)
fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
if (ret == -ENOIOCTLCMD)
- ret = -EINVAL;
+ ret = -ENOTTY;
mutex_unlock(&video->mutex);
@@ -723,7 +723,7 @@ isp_video_try_format(struct file *file, void *fh, struct v4l2_format *format)
fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
if (ret)
- return ret == -ENOIOCTLCMD ? -EINVAL : ret;
+ return ret == -ENOIOCTLCMD ? -ENOTTY : ret;
isp_video_mbus_to_pix(video, &fmt.format, &format->fmt.pix);
return 0;
@@ -744,7 +744,7 @@ isp_video_cropcap(struct file *file, void *fh, struct v4l2_cropcap *cropcap)
ret = v4l2_subdev_call(subdev, video, cropcap, cropcap);
mutex_unlock(&video->mutex);
- return ret == -ENOIOCTLCMD ? -EINVAL : ret;
+ return ret == -ENOIOCTLCMD ? -ENOTTY : ret;
}
static int
@@ -771,7 +771,7 @@ isp_video_get_crop(struct file *file, void *fh, struct v4l2_crop *crop)
format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &format);
if (ret < 0)
- return ret == -ENOIOCTLCMD ? -EINVAL : ret;
+ return ret == -ENOIOCTLCMD ? -ENOTTY : ret;
crop->c.left = 0;
crop->c.top = 0;
@@ -796,7 +796,7 @@ isp_video_set_crop(struct file *file, void *fh, struct v4l2_crop *crop)
ret = v4l2_subdev_call(subdev, video, s_crop, crop);
mutex_unlock(&video->mutex);
- return ret == -ENOIOCTLCMD ? -EINVAL : ret;
+ return ret == -ENOIOCTLCMD ? -ENOTTY : ret;
}
static int
--
1.7.12
At commit 07d106d0, Linus pointed out that ENOIOCTLCMD should be
translated as ENOTTY to user mode.
Cc: Martin Schwidefsky <[email protected]>
Cc: Heiko Carstens <[email protected]>
Cc: [email protected]
Cc: [email protected] (open list:S390)
Signed-off-by: Wanlong Gao <[email protected]>
---
drivers/s390/block/dasd_ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/s390/block/dasd_ioctl.c b/drivers/s390/block/dasd_ioctl.c
index cceae70..809a89b 100644
--- a/drivers/s390/block/dasd_ioctl.c
+++ b/drivers/s390/block/dasd_ioctl.c
@@ -501,7 +501,7 @@ int dasd_ioctl(struct block_device *bdev, fmode_t mode,
if (base->discipline->ioctl) {
rc = base->discipline->ioctl(block, cmd, argp);
if (rc == -ENOIOCTLCMD)
- rc = -EINVAL;
+ rc = -ENOTTY;
} else
rc = -EINVAL;
}
--
1.7.12
At commit 07d106d0, Linus pointed out that ENOIOCTLCMD should be
translated as ENOTTY to user mode.
Cc: Mauro Carvalho Chehab <[email protected]>
Cc: [email protected]
Signed-off-by: Wanlong Gao <[email protected]>
---
drivers/media/dvb/dvb-core/dvbdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/dvb/dvb-core/dvbdev.c b/drivers/media/dvb/dvb-core/dvbdev.c
index 39eab73..d33101a 100644
--- a/drivers/media/dvb/dvb-core/dvbdev.c
+++ b/drivers/media/dvb/dvb-core/dvbdev.c
@@ -420,7 +420,7 @@ int dvb_usercopy(struct file *file,
/* call driver */
mutex_lock(&dvbdev_mutex);
if ((err = func(file, cmd, parg)) == -ENOIOCTLCMD)
- err = -EINVAL;
+ err = -ENOTTY;
mutex_unlock(&dvbdev_mutex);
if (err < 0)
--
1.7.12
At commit 07d106d0, Linus pointed out that ENOIOCTLCMD should be
translated as ENOTTY to user mode.
For example:
fd = open("/dev/tty", O_RDWR);
ioctl(fd, -1, &argp);
then the errno should be ENOTTY but not EINVAL.
Cc: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Wanlong Gao <[email protected]>
---
drivers/tty/tty_io.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index b425c79..720c408f 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2756,7 +2756,7 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
if (ld->ops->ioctl) {
retval = ld->ops->ioctl(tty, file, cmd, arg);
if (retval == -ENOIOCTLCMD)
- retval = -EINVAL;
+ retval = -ENOTTY;
}
tty_ldisc_deref(ld);
return retval;
--
1.7.12
On Mon, Aug 27, 2012 at 03:23:16PM +0800, Wanlong Gao wrote:
> At commit 07d106d0, Linus pointed out that ENOIOCTLCMD should be
> translated as ENOTTY to user mode.
>
[...]
> Signed-off-by: Wanlong Gao <[email protected]>
> ---
> drivers/s390/block/dasd_ioctl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/s390/block/dasd_ioctl.c b/drivers/s390/block/dasd_ioctl.c
> index cceae70..809a89b 100644
> --- a/drivers/s390/block/dasd_ioctl.c
> +++ b/drivers/s390/block/dasd_ioctl.c
> @@ -501,7 +501,7 @@ int dasd_ioctl(struct block_device *bdev, fmode_t mode,
> if (base->discipline->ioctl) {
> rc = base->discipline->ioctl(block, cmd, argp);
> if (rc == -ENOIOCTLCMD)
> - rc = -EINVAL;
> + rc = -ENOTTY;
> } else
> rc = -EINVAL;
> }
Thanks, but you missed the else path. I'm going to commit the patch below
unless Stefan has any objections:
>From dac16bd8b314dc6f3f4e6815feab199fdfc8cddd Mon Sep 17 00:00:00 2001
From: Heiko Carstens <[email protected]>
Date: Mon, 27 Aug 2012 10:59:42 +0200
Subject: [PATCH] s390/dasd: fix ioctl return value
For unimplemented ioctls the dasd driver should return -ENOTTY.
Reported-by: Wanlong Gao <[email protected]>
Signed-off-by: Heiko Carstens <[email protected]>
---
drivers/s390/block/dasd_eckd.c | 2 +-
drivers/s390/block/dasd_ioctl.c | 7 ++-----
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
index 40a826a..2fb2b9e 100644
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -3804,7 +3804,7 @@ dasd_eckd_ioctl(struct dasd_block *block, unsigned int cmd, void __user *argp)
case BIODASDSYMMIO:
return dasd_symm_io(device, argp);
default:
- return -ENOIOCTLCMD;
+ return -ENOTTY;
}
}
diff --git a/drivers/s390/block/dasd_ioctl.c b/drivers/s390/block/dasd_ioctl.c
index cceae70..654c692 100644
--- a/drivers/s390/block/dasd_ioctl.c
+++ b/drivers/s390/block/dasd_ioctl.c
@@ -498,12 +498,9 @@ int dasd_ioctl(struct block_device *bdev, fmode_t mode,
break;
default:
/* if the discipline has an ioctl method try it. */
- if (base->discipline->ioctl) {
+ rc = -ENOTTY;
+ if (base->discipline->ioctl)
rc = base->discipline->ioctl(block, cmd, argp);
- if (rc == -ENOIOCTLCMD)
- rc = -EINVAL;
- } else
- rc = -EINVAL;
}
dasd_put_device(base);
return rc;
--
1.7.11.5
[email protected] wrote on 2012-08-27 11:03:55:
[..]
>
> Thanks, but you missed the else path. I'm going to commit the patch
below
> unless Stefan has any objections:
>
> From dac16bd8b314dc6f3f4e6815feab199fdfc8cddd Mon Sep 17 00:00:00 2001
> From: Heiko Carstens <[email protected]>
> Date: Mon, 27 Aug 2012 10:59:42 +0200
> Subject: [PATCH] s390/dasd: fix ioctl return value
>
> For unimplemented ioctls the dasd driver should return -ENOTTY.
>
> Reported-by: Wanlong Gao <[email protected]>
> Signed-off-by: Heiko Carstens <[email protected]>
> ---
> drivers/s390/block/dasd_eckd.c | 2 +-
> drivers/s390/block/dasd_ioctl.c | 7 ++-----
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/s390/block/dasd_eckd.c
b/drivers/s390/block/dasd_eckd.c
> index 40a826a..2fb2b9e 100644
> --- a/drivers/s390/block/dasd_eckd.c
> +++ b/drivers/s390/block/dasd_eckd.c
> @@ -3804,7 +3804,7 @@ dasd_eckd_ioctl(struct dasd_block *block,
> unsigned int cmd, void __user *argp)
> case BIODASDSYMMIO:
> return dasd_symm_io(device, argp);
> default:
> - return -ENOIOCTLCMD;
> + return -ENOTTY;
> }
> }
>
> diff --git a/drivers/s390/block/dasd_ioctl.c
b/drivers/s390/block/dasd_ioctl.c
> index cceae70..654c692 100644
> --- a/drivers/s390/block/dasd_ioctl.c
> +++ b/drivers/s390/block/dasd_ioctl.c
> @@ -498,12 +498,9 @@ int dasd_ioctl(struct block_device *bdev, fmode_t
mode,
> break;
> default:
> /* if the discipline has an ioctl method try it. */
> - if (base->discipline->ioctl) {
> + rc = -ENOTTY;
> + if (base->discipline->ioctl)
> rc = base->discipline->ioctl(block, cmd, argp);
> - if (rc == -ENOIOCTLCMD)
> - rc = -EINVAL;
> - } else
> - rc = -EINVAL;
> }
> dasd_put_device(base);
> return rc;
> --
> 1.7.11.5
>
The patch is looking fine to me, thanks.
Mit freundlichen Gr??en / Kind regards
Stefan Weinhuber
On Mon, 27 Aug 2012 15:23:12 +0800
Wanlong Gao <[email protected]> wrote:
> At commit 07d106d0, Linus pointed out that ENOIOCTLCMD should be
> translated as ENOTTY to user mode.
> For example:
> fd = open("/dev/tty", O_RDWR);
> ioctl(fd, -1, &argp);
>
> then the errno should be ENOTTY but not EINVAL.
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Wanlong Gao <[email protected]>
Acked-by: Alan Cox <[email protected]>
From: Wanlong Gao <[email protected]>
Date: Mon, 27 Aug 2012 15:23:13 +0800
> At commit 07d106d0, Linus pointed out that ENOIOCTLCMD should be
> translated as ENOTTY to user mode.
>
> Cc: "David S. Miller" <[email protected]>
> Cc: [email protected]
> Signed-off-by: Wanlong Gao <[email protected]>
Applied to net-next, thanks.