isatty false positives and device state clobbering

isatty false positives and device state clobbering

[Rich Felker]

As can be seen by strace, the TCGETS ioctl used by
isatty/fdopen/__stdout_write to determine whether a file descriptor is
a terminal is aliased by the SNDCTL_TMR_TIMEBASE ioctl for OSS sound
devices. This is an utterly stupid legacy mistake, but it means the
ioctl could spuriously succeed and change the state (time base) of a
midi sequencer device when it's intended just to query whether the
device is a terminal.

Even though it's unlikely to arise in practice, I'd like to find a
clean solution to the problem. I see two general approaches:

1. Use fstat first and blacklist the sound device major before using
   the ioctl, or even hard-code a list of tty majors and determine
   positive tty status by device number.

2. Find an ioctl that doesn't clash with OSS (or anything else, but
   OSS is the only driver I know with this bogus ioctl space collision
   with ttys) and that doesn't change the tty state, and use that
   instead.

I strongly prefer strategy 2; hard-coding device numbers seems really
backwards and precludes portability of source/binaries to platforms
that provide identical names and userspace API/ABI but different
device numbering.

OSS seems to use the range 0x5401 to 0x5408, so some possible
candidates for strategy 2 seem to be:

#define TIOCGPGRP       0x540F
#define TIOCOUTQ        0x5411
#define TIOCGWINSZ      0x5413
#define FIONREAD        0x541B

Perhaps TIOCGPGRP is best if it works for ttys that aren't the
controlling tty for a process group, since it corresponds to a
standard POSIX feature and would need to be present on any system
where the tcgetpgrp() is implemented via ioctl. The others are
nonstandard but widely supported extensions for querying terminal
buffer state and window size.

It's also worth checking whether these are defined differently on any
particular archs (e.g. mips, uhg) and whether the definitions there
might clash with OSS ioctl numbers, in which case selecting a
different one would be preferable.

Rich

Re: isatty false positives and device state clobbering

[Solar Designer]

On Fri, Jan 30, 2015 at 10:29:53PM -0500, Rich Felker wrote:
> As can be seen by strace, the TCGETS ioctl used by
> isatty/fdopen/__stdout_write to determine whether a file descriptor is
> a terminal is aliased by the SNDCTL_TMR_TIMEBASE ioctl for OSS sound
> devices.

FWIW, I think I was the one to implement this reporting in strace:

http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/strace/Attic/strace-20020608-owl-ioctl.diff?rev=1.1;content-type=text%2Fx-cvsweb-markup

(and this was later upstreamed), specifically because of the historical
mistake you describe.

IIRC, strace would say simply SNDCTL_TMR_TIMEBASE before this fix, even
though in most programs TCGETS was meant.

It's nice that you're trying to deal with another aspect of this issue
now... but not so nice that it exists in the first place.

Alexander

Re: isatty false positives and device state clobbering

[Rich Felker]

On Fri, Jan 30, 2015 at 10:29:53PM -0500, Rich Felker wrote:
> OSS seems to use the range 0x5401 to 0x5408, so some possible
> candidates for strategy 2 seem to be:
> 
> #define TIOCGPGRP       0x540F
> #define TIOCOUTQ        0x5411
> #define TIOCGWINSZ      0x5413
> #define FIONREAD        0x541B
> 
> Perhaps TIOCGPGRP is best if it works for ttys that aren't the
> controlling tty for a process group, since it corresponds to a
> standard POSIX feature and would need to be present on any system
> where the tcgetpgrp() is implemented via ioctl. The others are
> nonstandard but widely supported extensions for querying terminal
> buffer state and window size.
> 
> It's also worth checking whether these are defined differently on any
> particular archs (e.g. mips, uhg) and whether the definitions there
> might clash with OSS ioctl numbers, in which case selecting a
> different one would be preferable.

I think TIOCGPGRP looks safe against clashes:

$ grep TIOCGPGRP arch/*/bits/ioctl.h
arch/arm/bits/ioctl.h:#define TIOCGPGRP 0x540F
arch/i386/bits/ioctl.h:#define TIOCGPGRP        0x540F
arch/microblaze/bits/ioctl.h:#define TIOCGPGRP  0x540F
arch/mips/bits/ioctl.h:#define TIOCGPGRP        _IOR('t', 119, int)
arch/or1k/bits/ioctl.h:#define TIOCGPGRP        0x540F
arch/powerpc/bits/ioctl.h:#define TIOCGPGRP     _IOR('t', 119, int)
arch/sh/bits/ioctl.h:#define TIOCGPGRP           _IOR('t', 119, int)
arch/x32/bits/ioctl.h:#define TIOCGPGRP 0x540F
arch/x86_64/bits/ioctl.h:#define TIOCGPGRP      0x540F

Unfortunately, per POSIX:

    The tcgetpgrp() function shall fail if:

    [ENOTTY]
    The calling process does not have a controlling terminal, or the
    file is not the controlling terminal.

Whether it actually does or not, this function (and the underlying
ioctl, if it's implemented as an ioctl) is supposed to return ENOTTY
when the caller does not have a controlling terminal. So it doesn't
seem like it can provide the functionality we need.

Fortunately, TIOCGWINSZ, FIONREAD, and TIOCOUTQ also _seem_ to avoid
clashes (although mips has some wacky numbering for them that would
probably warrant further checks -- use of 'F' and 't' ioctl classes
instead of 'T') so it's probably a matter of checking that these are
supported on other systems we might care about (BSD Linux emulation?)
and picking one.

Rich

Re: isatty false positives and device state clobbering

[M Farkas-Dyck]

On 30/01/2015, Rich Felker <dalias@libc.org> wrote:
> Fortunately, TIOCGWINSZ, FIONREAD, and TIOCOUTQ also _seem_ to avoid
> clashes (although mips has some wacky numbering for them that would
> probably warrant further checks -- use of 'F' and 't' ioctl classes
> instead of 'T') so it's probably a matter of checking that these are
> supported on other systems we might care about (BSD Linux emulation?)
> and picking one.

These are so defined in OpenBSD Linux compat layer:
0x540F TIOCGPGRP
0x5411 TIOCOUTQ
0x5413 TIOCGWINSZ
0x541B FIONREAD

Re: isatty false positives and device state clobbering

[Rich Felker]

On Fri, Jan 30, 2015 at 11:01:21PM -0500, Rich Felker wrote:
> Fortunately, TIOCGWINSZ, FIONREAD, and TIOCOUTQ also _seem_ to avoid
> clashes (although mips has some wacky numbering for them that would
> probably warrant further checks -- use of 'F' and 't' ioctl classes
> instead of 'T') so it's probably a matter of checking that these are
> supported on other systems we might care about (BSD Linux emulation?)
> and picking one.

On further thought, it seems plausible that someone might want to
implement FIONREAD/TIOCOUTQ for non-tty devices that have input or
output queues, so relying on them failing for non-tty devices seems
wrong. On the other hand, "window size" seems unlikely to be
reasonable for anything but a tty. So I think TIOCGWINSZ is the best
candidate so far. Any other ideas?

Rich