[regression] ENOTTY returned for tty fds

[regression] ENOTTY returned for tty fds

[stsp]

Hi!

I've noticed that one of my progs failed
to work with serial ports. To distinguish
between real serial ports and PTS nodes,
it tried TIOCMBIC, checking errno for EINVAL.

Obviously that behaviour was changed
and now ENOTTY is returned. Besides
being backward-incompatible, I think this
is also wrong because isatty() returns 1
on such FDs.

Here's the test-case:
socat PTY,link=/tmp/ttyS20,raw,echo=0,b9600 
PTY,link=/tmp/ttyS21,raw,echo=0,b9600

Then run this program:

#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <errno.h>
#include <sys/ioctl.h>
#include <unistd.h>

int main() {
   int fd, err;
   int data = TIOCM_DTR | TIOCM_RTS;
   fd = open("/tmp/ttyS20", O_NONBLOCK);
   if (fd == -1) {
     perror("open()");
     return 1;
   }
   err = ioctl(fd, TIOCMBIC, &data);
   if (err && errno == ENOTTY && isatty(fd))
     printf("Test FAILED: ENOTTY for tty fd\n");
   return 0;
}

Re: [regression] ENOTTY returned for tty fds

[Greg KH]

On Sun, Jul 21, 2024 at 02:52:31PM +0300, stsp wrote:
> Hi!
> 
> I've noticed that one of my progs failed
> to work with serial ports. To distinguish
> between real serial ports and PTS nodes,
> it tried TIOCMBIC, checking errno for EINVAL.
> 
> Obviously that behaviour was changed
> and now ENOTTY is returned. Besides
> being backward-incompatible, I think this
> is also wrong because isatty() returns 1
> on such FDs.
> 
> Here's the test-case:
> socat PTY,link=/tmp/ttyS20,raw,echo=0,b9600
> PTY,link=/tmp/ttyS21,raw,echo=0,b9600
> 
> Then run this program:
> 
> #include <stdio.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <sys/ioctl.h>
> #include <unistd.h>
> 
> int main() {
>   int fd, err;
>   int data = TIOCM_DTR | TIOCM_RTS;
>   fd = open("/tmp/ttyS20", O_NONBLOCK);
>   if (fd == -1) {
>     perror("open()");
>     return 1;
>   }
>   err = ioctl(fd, TIOCMBIC, &data);
>   if (err && errno == ENOTTY && isatty(fd))
>     printf("Test FAILED: ENOTTY for tty fd\n");
>   return 0;
> }
> 
> 

What caused this change/regression?  And does any real-world programs
rely on this?  What exactly are you trying to determine with this ioctl
test?  Is there a different way to determine that?

thanks,

greg k-h

Re: [regression] ENOTTY returned for tty fds

[stsp]

24.07.2024 09:51, Greg KH пишет:
> What caused this change/regression?

I have absolutely no idea.
I've found it by debugging userspace,
and wrote a test-case to make sure the
problem is not in user-space.


>    And does any real-world programs
> rely on this?

dosemu


>    What exactly are you trying to determine with this ioctl
> test?

Whether it is a PTS (Pseudo-Tty-Slave), or
a real comport with MSR signalling.


>    Is there a different way to determine that?
I am not aware of any "canonical" way
of determining this. Maybe you tell me. :)
So far the only fix I know, is to stop checking
errno. But you return ENOTTY for a tty-associated
fd (isatty(fd)==1), so I believe this is a
bug in a kernel.

Re: [regression] ENOTTY returned for tty fds

[Greg KH]

On Wed, Jul 24, 2024 at 11:07:32AM +0300, stsp wrote:
> 24.07.2024 09:51, Greg KH пишет:
> > What caused this change/regression?
> 
> I have absolutely no idea.
> I've found it by debugging userspace,
> and wrote a test-case to make sure the
> problem is not in user-space.

So this has always worked this way?  Or has it changed?  If changed,
when did it work before?

> >    And does any real-world programs
> > rely on this?
> 
> dosemu

It does this today or wants to do this in the future?

> >    What exactly are you trying to determine with this ioctl
> > test?
> 
> Whether it is a PTS (Pseudo-Tty-Slave), or
> a real comport with MSR signalling.

Why is that needed?  And why not do it how other programs (like stty)
does it?

> >    Is there a different way to determine that?
> I am not aware of any "canonical" way
> of determining this. Maybe you tell me. :)
> So far the only fix I know, is to stop checking
> errno. But you return ENOTTY for a tty-associated
> fd (isatty(fd)==1), so I believe this is a
> bug in a kernel.

isatty() is a libc provided function, not a kernel call.

thanks,

greg k-h

Re: [regression] ENOTTY returned for tty fds

[stsp]

24.07.2024 12:08, Greg KH пишет:
> On Wed, Jul 24, 2024 at 11:07:32AM +0300, stsp wrote:
>> 24.07.2024 09:51, Greg KH пишет:
>>> What caused this change/regression?
>> I have absolutely no idea.
>> I've found it by debugging userspace,
>> and wrote a test-case to make sure the
>> problem is not in user-space.
> So this has always worked this way?  Or has it changed?  If changed,
> when did it work before?

OK, I did some extensive digging, and
now am pretty sure its this one:

commit 1b8b20868a6d64cfe8174a21b25b74367bdf0560
Author:     Johan Hovold <johan@kernel.org>
AuthorDate: Wed Apr 7 11:52:02 2021 +0200
Commit:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CommitDate: Sat Apr 10 10:36:34 2021 +0200

     tty: fix return value for unsupported ioctls

     Drivers should return -ENOTTY ("Inappropriate I/O control operation")
     when an ioctl isn't supported, while -EINVAL is used for invalid
     arguments.

>>>     And does any real-world programs
>>> rely on this?
>> dosemu
> It does this today or wants to do this in the future?

It does so since 2003/03/15, according
to a change-log (so for 21 year now).
Adding Herbert Xu to CC as an author of
that feature.

>>>     What exactly are you trying to determine with this ioctl
>>> test?
>> Whether it is a PTS (Pseudo-Tty-Slave), or
>> a real comport with MSR signalling.
> Why is that needed?

To fake modem status lines, mustly DCD.


>    And why not do it how other programs (like stty)
> does it?

I am not sure stty is interested in faking
modem status lines. It doesn't seem to be
using TIOCM ioctls at all.

>>>     Is there a different way to determine that?
>> I am not aware of any "canonical" way
>> of determining this. Maybe you tell me. :)
>> So far the only fix I know, is to stop checking
>> errno. But you return ENOTTY for a tty-associated
>> fd (isatty(fd)==1), so I believe this is a
>> bug in a kernel.
> isatty() is a libc provided function, not a kernel call.
It seems to be using TCGETS ioctl() and looks
for it to not return an error.
I still think returning ENOTTY where TCGETS
succeeds, is more than strange.
I looked at the tty code of current linux kernel,
and it returns EOPNOTSUPP in most such cases.
At least that makes sense, but ENOTTY?

Re: [regression] ENOTTY returned for tty fds

[Greg KH]

On Wed, Jul 24, 2024 at 12:55:22PM +0300, stsp wrote:
> 24.07.2024 12:08, Greg KH пишет:
> > On Wed, Jul 24, 2024 at 11:07:32AM +0300, stsp wrote:
> > > 24.07.2024 09:51, Greg KH пишет:
> > > > What caused this change/regression?
> > > I have absolutely no idea.
> > > I've found it by debugging userspace,
> > > and wrote a test-case to make sure the
> > > problem is not in user-space.
> > So this has always worked this way?  Or has it changed?  If changed,
> > when did it work before?
> 
> OK, I did some extensive digging, and
> now am pretty sure its this one:
> 
> commit 1b8b20868a6d64cfe8174a21b25b74367bdf0560
> Author:     Johan Hovold <johan@kernel.org>
> AuthorDate: Wed Apr 7 11:52:02 2021 +0200
> Commit:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> CommitDate: Sat Apr 10 10:36:34 2021 +0200
> 
>     tty: fix return value for unsupported ioctls
> 
>     Drivers should return -ENOTTY ("Inappropriate I/O control operation")
>     when an ioctl isn't supported, while -EINVAL is used for invalid
>     arguments.
> 
> > > >     And does any real-world programs
> > > > rely on this?
> > > dosemu
> > It does this today or wants to do this in the future?
> 
> It does so since 2003/03/15, according
> to a change-log (so for 21 year now).
> Adding Herbert Xu to CC as an author of
> that feature.
> 
> > > >     What exactly are you trying to determine with this ioctl
> > > > test?
> > > Whether it is a PTS (Pseudo-Tty-Slave), or
> > > a real comport with MSR signalling.
> > Why is that needed?
> 
> To fake modem status lines, mustly DCD.
> 
> 
> >    And why not do it how other programs (like stty)
> > does it?
> 
> I am not sure stty is interested in faking
> modem status lines. It doesn't seem to be
> using TIOCM ioctls at all.
> 
> > > >     Is there a different way to determine that?
> > > I am not aware of any "canonical" way
> > > of determining this. Maybe you tell me. :)
> > > So far the only fix I know, is to stop checking
> > > errno. But you return ENOTTY for a tty-associated
> > > fd (isatty(fd)==1), so I believe this is a
> > > bug in a kernel.
> > isatty() is a libc provided function, not a kernel call.
> It seems to be using TCGETS ioctl() and looks
> for it to not return an error.
> I still think returning ENOTTY where TCGETS
> succeeds, is more than strange.
> I looked at the tty code of current linux kernel,
> and it returns EOPNOTSUPP in most such cases.
> At least that makes sense, but ENOTTY?

-ENOTTY is the documented result of invalid ioctl arguments sent, I am
pretty sure POSIX requires this somewhere.  So this was fixing a
requirement here...

thanks,

greg k-h

Re: [regression] ENOTTY returned for tty fds

[Herbert Xu]

On Wed, Jul 24, 2024 at 12:15:39PM +0200, Greg KH wrote:
>
> -ENOTTY is the documented result of invalid ioctl arguments sent, I am
> pretty sure POSIX requires this somewhere.  So this was fixing a
> requirement here...

POSIX does not specify this at all:

https://pubs.opengroup.org/onlinepubs/9699919799/functions/ioctl.html

I think this should be reverted as it breaks user-space code
that dates back to 2003.

ENOTTY doesn't even make sense anyway as it literally means "is not a
terminal" and pseudo-ttys certainly are terminals.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [regression] ENOTTY returned for tty fds

[Herbert Xu]

On Wed, Jul 24, 2024 at 06:53:00PM +0800, Herbert Xu wrote:
> On Wed, Jul 24, 2024 at 12:15:39PM +0200, Greg KH wrote:
> >
> > -ENOTTY is the documented result of invalid ioctl arguments sent, I am
> > pretty sure POSIX requires this somewhere.  So this was fixing a
> > requirement here...
> 
> POSIX does not specify this at all:
> 
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/ioctl.html

In fact it says:

If an underlying device driver detects an error, then ioctl() shall fail if:

[EINVAL]
The request or arg argument is not valid for this device.

[ENOTTY]
The file associated with the fildes argument is not a STREAMS device that accepts control functions.

Of course this is all moot since POSIX only specifies ioctl(2)
for STREAMS devices, but this patch in question is literally
going against the woring here.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [regression] ENOTTY returned for tty fds

[Greg KH]

On Wed, Jul 24, 2024 at 06:58:22PM +0800, Herbert Xu wrote:
> On Wed, Jul 24, 2024 at 06:53:00PM +0800, Herbert Xu wrote:
> > On Wed, Jul 24, 2024 at 12:15:39PM +0200, Greg KH wrote:
> > >
> > > -ENOTTY is the documented result of invalid ioctl arguments sent, I am
> > > pretty sure POSIX requires this somewhere.  So this was fixing a
> > > requirement here...
> > 
> > POSIX does not specify this at all:
> > 
> > https://pubs.opengroup.org/onlinepubs/9699919799/functions/ioctl.html
> 
> In fact it says:
> 
> If an underlying device driver detects an error, then ioctl() shall fail if:
> 
> [EINVAL]
> The request or arg argument is not valid for this device.
> 
> [ENOTTY]
> The file associated with the fildes argument is not a STREAMS device that accepts control functions.
> 
> Of course this is all moot since POSIX only specifies ioctl(2)
> for STREAMS devices, but this patch in question is literally
> going against the woring here.

That's going to be going against a lot of existing kernel code then.
-ENOTTY is the default action of almost all ioctl handlers when the
command is not a valid one, sorry.

thanks,

greg k-h

Re: [regression] ENOTTY returned for tty fds

[Johan Hovold]

[ For some reason I never received Herbert's replies so I didn't see
  them until today. ]

On Wed, Jul 31, 2024 at 12:25:43PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jul 24, 2024 at 06:58:22PM +0800, Herbert Xu wrote:
> > On Wed, Jul 24, 2024 at 06:53:00PM +0800, Herbert Xu wrote:
> > > On Wed, Jul 24, 2024 at 12:15:39PM +0200, Greg KH wrote:
> > > >
> > > > -ENOTTY is the documented result of invalid ioctl arguments sent, I am
> > > > pretty sure POSIX requires this somewhere.  So this was fixing a
> > > > requirement here...
> > > 
> > > POSIX does not specify this at all:
> > > 
> > > https://pubs.opengroup.org/onlinepubs/9699919799/functions/ioctl.html
> > 
> > In fact it says:
> > 
> > If an underlying device driver detects an error, then ioctl() shall fail if:
> > 
> > [EINVAL]
> > The request or arg argument is not valid for this device.
> > 
> > [ENOTTY]
> > The file associated with the fildes argument is not a STREAMS device that accepts control functions.
> > 
> > Of course this is all moot since POSIX only specifies ioctl(2)
> > for STREAMS devices, but this patch in question is literally
> > going against the woring here.
> 
> That's going to be going against a lot of existing kernel code then.
> -ENOTTY is the default action of almost all ioctl handlers when the
> command is not a valid one, sorry.

This has been discussed at length before, and Linus made it clear that
ENOTTY is the right errno for unsupported ioctls, for example, here:

	https://lore.kernel.org/lkml/BANLkTi=6W0quy1M71UapwKDe97E67b4EiA@mail.gmail.com/

The tty code has been returning this since 2012 and commit bbb63c514a34
("drivers:tty:fix up ENOIOCTLCMD error handling") except for a few cases
that were missed and that I fixed up in 2021.

If this breaks real applications me may need to revert to EINVAL for the
affected ioctls however. Apparently, dosemu hasn't been
updated since 2013, but it looks like it indeed expects EINVAL for
unsupported TIOCMBIC.

Johan