* Re: [LTP] [PATCH] ioctl01: change the errno to ENOTTY when passed an invalid command [not found] <1351742419-30299-1-git-send-email-gaowanlong@cn.fujitsu.com> @ 2012-11-01 8:05 ` Jan Stancek 2012-11-01 8:15 ` Wanlong Gao 0 siblings, 1 reply; 4+ messages in thread From: Jan Stancek @ 2012-11-01 8:05 UTC (permalink / raw) To: Wanlong Gao; +Cc: shyju pv, sanil kumar, max maxiansheng, Mike Frysinger, LTP ----- Original Message ----- > From: "Wanlong Gao" <gaowanlong@cn.fujitsu.com> > To: "LTP" <ltp-list@lists.sourceforge.net> > Cc: "sanil kumar" <sanil.kumar@huawei.com>, "Mike Frysinger" <vapier@gentoo.org>, "shyju pv" <shyju.pv@huawei.com>, > "max maxiansheng" <max.maxiansheng@huawei.com> > Sent: Thursday, 1 November, 2012 5:00:19 AM > Subject: [LTP] [PATCH] ioctl01: change the errno to ENOTTY when passed an invalid command > > As linus said at the below commit, > commit 07d106d0a33d6063d2061305903deb02489eba20 > Author: Linus Torvalds <torvalds@linux-foundation.org> > 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 <torvalds@linux-foundation.org> > > Return ENOTTY is right. And with the tty driver change with below > commit, > commit bbb63c514a3464342967237a51a21ea8f61ab951 > Author: Wanlong Gao <gaowanlong@cn.fujitsu.com> > 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. > > Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> > Acked-by: Alan Cox <alan@linux.intel.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > The tty driver will return the right ENOTTY in upstream kernel when > passed a invalid ioctl command, so we fixed the LTP test case to > suit this return value change. > --- > testcases/kernel/syscalls/ioctl/ioctl01.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c > b/testcases/kernel/syscalls/ioctl/ioctl01.c > index 8b044e7..ef64896 100644 > --- a/testcases/kernel/syscalls/ioctl/ioctl01.c > +++ b/testcases/kernel/syscalls/ioctl/ioctl01.c > @@ -87,7 +87,7 @@ struct test_case_t { > &fd, TCGETA, (struct termio *)-1, EFAULT}, > /* command is invalid */ > { > - &fd, INVAL_IOCTL, &termio, EINVAL}, > + &fd, INVAL_IOCTL, &termio, ENOTTY}, Won't this break on older kernels? Can we test for kernel version with tst_kvercmp()? Regards, Jan > /* file descriptor is for a regular file */ > { > &fd1, TCGETA, &termio, ENOTTY}, > -- > 1.8.0 > > > ------------------------------------------------------------------------------ > Everyone hates slow websites. So do we. > Make your web apps faster with AppDynamics > Download AppDynamics Lite for free today: > http://p.sf.net/sfu/appdyn_sfd2d_oct > _______________________________________________ > Ltp-list mailing list > Ltp-list@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/ltp-list > ------------------------------------------------------------------------------ Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct _______________________________________________ Ltp-list mailing list Ltp-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ltp-list ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [LTP] [PATCH] ioctl01: change the errno to ENOTTY when passed an invalid command 2012-11-01 8:05 ` [LTP] [PATCH] ioctl01: change the errno to ENOTTY when passed an invalid command Jan Stancek @ 2012-11-01 8:15 ` Wanlong Gao 2012-11-01 9:55 ` Jan Stancek 0 siblings, 1 reply; 4+ messages in thread From: Wanlong Gao @ 2012-11-01 8:15 UTC (permalink / raw) To: Jan Stancek; +Cc: shyju pv, sanil kumar, max maxiansheng, Mike Frysinger, LTP On 11/01/2012 04:05 PM, Jan Stancek wrote: > > > ----- Original Message ----- >> From: "Wanlong Gao" <gaowanlong@cn.fujitsu.com> >> To: "LTP" <ltp-list@lists.sourceforge.net> >> Cc: "sanil kumar" <sanil.kumar@huawei.com>, "Mike Frysinger" <vapier@gentoo.org>, "shyju pv" <shyju.pv@huawei.com>, >> "max maxiansheng" <max.maxiansheng@huawei.com> >> Sent: Thursday, 1 November, 2012 5:00:19 AM >> Subject: [LTP] [PATCH] ioctl01: change the errno to ENOTTY when passed an invalid command >> >> As linus said at the below commit, >> commit 07d106d0a33d6063d2061305903deb02489eba20 >> Author: Linus Torvalds <torvalds@linux-foundation.org> >> 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 <torvalds@linux-foundation.org> >> >> Return ENOTTY is right. And with the tty driver change with below >> commit, >> commit bbb63c514a3464342967237a51a21ea8f61ab951 >> Author: Wanlong Gao <gaowanlong@cn.fujitsu.com> >> 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. >> >> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> >> Acked-by: Alan Cox <alan@linux.intel.com> >> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> >> The tty driver will return the right ENOTTY in upstream kernel when >> passed a invalid ioctl command, so we fixed the LTP test case to >> suit this return value change. >> --- >> testcases/kernel/syscalls/ioctl/ioctl01.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c >> b/testcases/kernel/syscalls/ioctl/ioctl01.c >> index 8b044e7..ef64896 100644 >> --- a/testcases/kernel/syscalls/ioctl/ioctl01.c >> +++ b/testcases/kernel/syscalls/ioctl/ioctl01.c >> @@ -87,7 +87,7 @@ struct test_case_t { >> &fd, TCGETA, (struct termio *)-1, EFAULT}, >> /* command is invalid */ >> { >> - &fd, INVAL_IOCTL, &termio, EINVAL}, >> + &fd, INVAL_IOCTL, &termio, ENOTTY}, > > Won't this break on older kernels? Can we test for kernel version with tst_kvercmp()? Surely will, this is also a trouble on my side. Does we treat this as a kernel bug or a kernel change? I think if we treat it as a kernel bug, we needn't check the kernel version, while if it just a kernel change, we need. And, What's your opinion about this? Thanks, Wanlong Gao > > Regards, > Jan > >> /* file descriptor is for a regular file */ >> { >> &fd1, TCGETA, &termio, ENOTTY}, >> -- >> 1.8.0 >> >> >> ------------------------------------------------------------------------------ >> Everyone hates slow websites. So do we. >> Make your web apps faster with AppDynamics >> Download AppDynamics Lite for free today: >> http://p.sf.net/sfu/appdyn_sfd2d_oct >> _______________________________________________ >> Ltp-list mailing list >> Ltp-list@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/ltp-list >> > ------------------------------------------------------------------------------ Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct _______________________________________________ Ltp-list mailing list Ltp-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ltp-list ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [LTP] [PATCH] ioctl01: change the errno to ENOTTY when passed an invalid command 2012-11-01 8:15 ` Wanlong Gao @ 2012-11-01 9:55 ` Jan Stancek 2012-11-01 10:05 ` Wanlong Gao 0 siblings, 1 reply; 4+ messages in thread From: Jan Stancek @ 2012-11-01 9:55 UTC (permalink / raw) To: gaowanlong; +Cc: shyju pv, sanil kumar, max maxiansheng, Mike Frysinger, LTP ----- Original Message ----- > From: "Wanlong Gao" <gaowanlong@cn.fujitsu.com> > To: "Jan Stancek" <jstancek@redhat.com> > Cc: "sanil kumar" <sanil.kumar@huawei.com>, "Mike Frysinger" <vapier@gentoo.org>, "shyju pv" <shyju.pv@huawei.com>, > "max maxiansheng" <max.maxiansheng@huawei.com>, "LTP" <ltp-list@lists.sourceforge.net> > Sent: Thursday, 1 November, 2012 9:15:08 AM > Subject: Re: [LTP] [PATCH] ioctl01: change the errno to ENOTTY when passed an invalid command > > >> diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c > >> b/testcases/kernel/syscalls/ioctl/ioctl01.c > >> index 8b044e7..ef64896 100644 > >> --- a/testcases/kernel/syscalls/ioctl/ioctl01.c > >> +++ b/testcases/kernel/syscalls/ioctl/ioctl01.c > >> @@ -87,7 +87,7 @@ struct test_case_t { > >> &fd, TCGETA, (struct termio *)-1, EFAULT}, > >> /* command is invalid */ > >> { > >> - &fd, INVAL_IOCTL, &termio, EINVAL}, > >> + &fd, INVAL_IOCTL, &termio, ENOTTY}, > > > > Won't this break on older kernels? Can we test for kernel version > > with tst_kvercmp()? > > Surely will, this is also a trouble on my side. > Does we treat this as a kernel bug or a kernel change? > I think if we treat it as a kernel bug, we needn't check the kernel > version, while if > it just a kernel change, we need. > And, What's your opinion about this? I'd go with kernel change, it worked the old way for years, there may be applications which rely on that. If we just change it to 'ENOTTY' it will start failing on older kernels and there are 2 choices: fix the kernel or stop using this testcase. And I'm not sure there are enough arguments to justify changing it in various distro's (stable) kernels. Is this commit backported to any upstream stable trees? Are there plans to do so? Regards, Jan ------------------------------------------------------------------------------ Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct _______________________________________________ Ltp-list mailing list Ltp-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ltp-list ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [LTP] [PATCH] ioctl01: change the errno to ENOTTY when passed an invalid command 2012-11-01 9:55 ` Jan Stancek @ 2012-11-01 10:05 ` Wanlong Gao 0 siblings, 0 replies; 4+ messages in thread From: Wanlong Gao @ 2012-11-01 10:05 UTC (permalink / raw) To: Jan Stancek; +Cc: shyju pv, sanil kumar, max maxiansheng, Mike Frysinger, LTP On 11/01/2012 05:55 PM, Jan Stancek wrote: > > > ----- Original Message ----- >> From: "Wanlong Gao" <gaowanlong@cn.fujitsu.com> >> To: "Jan Stancek" <jstancek@redhat.com> >> Cc: "sanil kumar" <sanil.kumar@huawei.com>, "Mike Frysinger" <vapier@gentoo.org>, "shyju pv" <shyju.pv@huawei.com>, >> "max maxiansheng" <max.maxiansheng@huawei.com>, "LTP" <ltp-list@lists.sourceforge.net> >> Sent: Thursday, 1 November, 2012 9:15:08 AM >> Subject: Re: [LTP] [PATCH] ioctl01: change the errno to ENOTTY when passed an invalid command >> >>>> diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c >>>> b/testcases/kernel/syscalls/ioctl/ioctl01.c >>>> index 8b044e7..ef64896 100644 >>>> --- a/testcases/kernel/syscalls/ioctl/ioctl01.c >>>> +++ b/testcases/kernel/syscalls/ioctl/ioctl01.c >>>> @@ -87,7 +87,7 @@ struct test_case_t { >>>> &fd, TCGETA, (struct termio *)-1, EFAULT}, >>>> /* command is invalid */ >>>> { >>>> - &fd, INVAL_IOCTL, &termio, EINVAL}, >>>> + &fd, INVAL_IOCTL, &termio, ENOTTY}, >>> >>> Won't this break on older kernels? Can we test for kernel version >>> with tst_kvercmp()? >> >> Surely will, this is also a trouble on my side. >> Does we treat this as a kernel bug or a kernel change? >> I think if we treat it as a kernel bug, we needn't check the kernel >> version, while if >> it just a kernel change, we need. >> And, What's your opinion about this? > > I'd go with kernel change, it worked the old way for years, there may be > applications which rely on that. If we just change it to 'ENOTTY' it will > start failing on older kernels and there are 2 choices: fix the kernel > or stop using this testcase. And I'm not sure there are enough arguments > to justify changing it in various distro's (stable) kernels. OK, agree. > > Is this commit backported to any upstream stable trees? Are there plans to do so? I didn't see any backport, it may because some drivers return the old value, we can't ensure to backport all of them, so I think there is no backprot plan. it's needless. Thanks, Wanlong Gao > > Regards, > Jan > ------------------------------------------------------------------------------ Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct _______________________________________________ Ltp-list mailing list Ltp-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ltp-list ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-11-01 10:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1351742419-30299-1-git-send-email-gaowanlong@cn.fujitsu.com>
2012-11-01 8:05 ` [LTP] [PATCH] ioctl01: change the errno to ENOTTY when passed an invalid command Jan Stancek
2012-11-01 8:15 ` Wanlong Gao
2012-11-01 9:55 ` Jan Stancek
2012-11-01 10:05 ` Wanlong Gao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox