Menu
▾
▴
Re: [LTP] [PATCH V2] ioctl01: change the errno to ENOTTY when passed an invalid command
|
From: Caspar Z. <ca...@ca...> - 2012-11-02 10:12:00
|
On 11/02/2012 04:51 PM, Wanlong Gao wrote:
> As linus said at the below commit,
> commit 07d106d0a33d6063d2061305903deb02489eba20
> Author: Linus Torvalds <tor...@li...>
> 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.
>
> Return ENOTTY is right. And with the tty driver change with below commit,
> commit bbb63c514a3464342967237a51a21ea8f61ab951
> Author: Wanlong Gao <gao...@cn...>
> Date: Mon Aug 27 15:23:12 2012 +0800
>
> drivers:tty:fix up ENOIOCTLCMD error handling
>
> 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.
>
> Suggested-by: Jan Stancek <jst...@re...>
> Signed-off-by: Wanlong Gao <gao...@cn...>
Reviewed-by: Caspar Zhang <ca...@ca...>
> ---
> testcases/kernel/syscalls/ioctl/ioctl01.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c b/testcases/kernel/syscalls/ioctl/ioctl01.c
> index 8b044e7..663e070 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl01.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl01.c
> @@ -66,7 +66,7 @@ void setup(void);
> void cleanup(void);
> void help(void);
>
> -int exp_enos[] = { EBADF, EFAULT, EINVAL, ENOTTY, EFAULT, 0 };
> +int exp_enos[] = { EBADF, EFAULT, ENOTTY, ENOTTY, EFAULT, 0 };
>
> int fd, fd1;
> int bfd = -1;
> @@ -86,8 +86,11 @@ struct test_case_t {
> {
> &fd, TCGETA, (struct termio *)-1, EFAULT},
> /* command is invalid */
> + /* This errno value was changed from EINVAL to ENOTTY
> + * by kernel commit 07d106d0 and bbb63c51
> + */
> {
> - &fd, INVAL_IOCTL, &termio, EINVAL},
> + &fd, INVAL_IOCTL, &termio, ENOTTY},
> /* file descriptor is for a regular file */
> {
> &fd1, TCGETA, &termio, ENOTTY},
> @@ -178,6 +181,11 @@ void setup(void) {
> /* make a temporary directory and cd to it */
> tst_tmpdir();
>
> + if (tst_kvercmp(3, 7, 0) < 0) {
> + exp_enos[2] = EINVAL;
> + TC[2].error = EINVAL;
> + }
> +
> /* create a temporary file */
> if ((fd1 = open("x", O_CREAT, 0777)) == -1)
> tst_resm(TFAIL|TERRNO, "Could not open test file");
>
|