Date: Sat, 19 Nov 2011 03:11:43 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Peter Wemm <peter@wemm.org> Cc: Garrett Cooper <yanegomi@gmail.com>, Giovanni Trematerra <gianni@freebsd.org>, freebsd-arch@freebsd.org Subject: Re: deprecated TIOCSPGRP and TIOCGPGRP ioctl command Message-ID: <20111119003059.V5050@besplex.bde.org> In-Reply-To: <CAGE5yCqSfJuemRgT7agTsydbBAJSrnAbyyoP3h%2BJ-JJMd1OAMw@mail.gmail.com> References: <CACfq09210zdar=%2BM40gLoVtZnwLda88T1FJBbLXiy9e3t8-9NQ@mail.gmail.com> <CAGE5yCor9c%2Bb7hZaF8NLdxc2F8PPfi0kTZFLXY5nvA%2Bbm1Ytuw@mail.gmail.com> <D2F25708-5E0B-4270-83F0-79286C5A581A@gmail.com> <CAGE5yCqSfJuemRgT7agTsydbBAJSrnAbyyoP3h%2BJ-JJMd1OAMw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
--0-1274663310-1321632703=:5050
Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed
Content-Transfer-Encoding: QUOTED-PRINTABLE
On Thu, 10 Nov 2011, Peter Wemm wrote:
> On Wed, Nov 9, 2011 at 5:52 PM, Garrett Cooper <yanegomi@gmail.com> wrote=
:
>> On Nov 9, 2011, at 4:49 PM, Peter Wemm wrote:
>>
>>> On Wed, Nov 9, 2011 at 3:39 PM, Giovanni Trematerra <gianni@freebsd.org=
> wrote:
>>>> Are they deprecated enough to be removed, now?
"They" (TIOC[SG]PGRP) aren't deprecated. They are fundamental ioctls
used to implement tc[sg]etpgrp(3). There are no specific syscalls for
the latter. The ioctls are probably deprecated, or even never supported
for direct use.
>>>> FYI FIFO doesn't support them.
These interfaces are for job control. What they mean for pipes and
fifos is unclear. I don't really understand what sys_pipe.c does for
them. It just makes TIOC[SG]PRGP the same as FIO[SG]ETOWN with the
arg negated. That makes some sense -- pgrps are normally represented
as negative pids, so if the underlying setown functions support both
prgps and processes as they should, using the usual sign hack to
distinguish pgrps for procs, then a sign change is needed to meet the
API of the under underlying functions.
>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>>> --- sys/kern/sys_pipe.c (revision 227233)
>>>> +++ sys/kern/sys_pipe.c (working copy)
>>>> @@ -1304,17 +1304,6 @@ pipe_ioctl(fp, cmd, data, active_cred, td)
>>>> =A0 =A0 =A0 =A0*(int *)data =3D fgetown(&mpipe->pipe_sigio);
>>>> =A0 =A0 =A0 =A0break;
>>>>
>>>> - =A0 /* This is deprecated, FIOSETOWN should be used instead. */
These "deprecated" comments were written by truckman. He fixed many
bugs in the [gs]etown interfaces. I made some comments about this. AFAIR
we only tried to fix only parts of [gs]etown including F_SETOWN (part of
fcntl()), and changes to the [gs]etpgroup interfaces were mainly side
effects, including the above fairly-misleading comment.
>>>> - =A0 case TIOCSPGRP:
>>>> - =A0 =A0 =A0 PIPE_UNLOCK(mpipe);
>>>> - =A0 =A0 =A0 error =3D fsetown(-(*(int *)data), &mpipe->pipe_sigio);
>>>> - =A0 =A0 =A0 goto out_unlocked;
>>>> -
>>>> - =A0 /* This is deprecated, FIOGETOWN should be used instead. */
>>>> - =A0 case TIOCGPGRP:
>>>> - =A0 =A0 =A0 *(int *)data =3D -fgetown(&mpipe->pipe_sigio);
>>>> - =A0 =A0 =A0 break;
>>>> -
>>>> =A0 =A0default:
>>>> =A0 =A0 =A0 =A0error =3D ENOTTY;
I think what these comments mean is:
"TIOC[SG]PGRP should only be used on ttys (and then only via
tc[gs]etpgrp()), but in case something abused it on pipes, keep doing
things the old way as above)." Diffs for the above between 2.2.9 and
3.final:
% Index: sys_pipe.c
% =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
% RCS file: /home/ncvs/src/sys/kern/sys_pipe.c,v
% retrieving revision 1.21
% retrieving revision 1.46.2.5
% diff -u -2 -r1.21 -r1.46.2.5
% --- sys_pipe.c=0911 Oct 1996 02:27:30 -0000=091.21
% +++ sys_pipe.c=0924 Mar 2000 00:48:57 -0000=091.46.2.5
% @@ -962,10 +952,18 @@
% =09=09return (0);
%=20
% -=09case TIOCSPGRP:
% -=09=09mpipe->pipe_pgid =3D *(int *)data;
% +=09case FIOSETOWN:
% +=09=09return (fsetown(*(int *)data, &mpipe->pipe_sigio));
% +
% +=09case FIOGETOWN:
% +=09=09*(int *)data =3D fgetown(mpipe->pipe_sigio);
% =09=09return (0);
%=20
% +=09/* This is deprecated, FIOSETOWN should be used instead. */
% +=09case TIOCSPGRP:
% +=09=09return (fsetown(-(*(int *)data), &mpipe->pipe_sigio));
% +
% +=09/* This is deprecated, FIOGETOWN should be used instead. */
% =09case TIOCGPGRP:
% -=09=09*(int *)data =3D mpipe->pipe_pgid;
% +=09=09*(int *)data =3D -fgetown(mpipe->pipe_sigio);
% =09=09return (0);
So the old way was to only support the TIOC* interfaces, presumably with
the same semantics as now modulo bugs that are hopefully fixed now.
Note that we usually get here, at least now, for fcntl(fd, F_[SG]ETOWN, ...=
).
This converts to the FIO[SG]ETOWN ioctls. Direct use of these ioctls is
or should be deprecated. The bugs hopefully fixed by truckman are caused
by the ioctls being device-specific while the fcntl()s are file-descriptor
or file-specific (I can never remember which). Old code used a single
pid or pgid per device. This more or less works for devices, but it
cannot work for multiple fd's or files open on the same device.
The fcntl() APIs support both processes and process groups encoded in
a single arg using the usual sign hack, but the old TIOC[SG]PGRP should
be limited by their very name to only supporting process groups. I
think they aren't actually limited. The current one, especially,
blindly flips the sign and does no range checking before getting to the
lower level function which accept all values (with originally negative
pgids being becoming positive and being interpreted as pids). That's
another reason why TIOC[SG]PGRP is garbage for pipes.
Another reason is that POSIX requires tcsetprgrp() "shall fail" if the
calling process does not have a controlling terminal, or the file is
not the controlling terminal, or the controlling terminal is no longer
associated with the session of the calling process. The error for
this is ENOTTY. Since FreeBSD's implementation of tcsetprgrp() uses
ioctl(), it will return this error automatically if the device is not
a tty, provided TIOC[SG]PRGP is not bogusly referenced in files for
non-tty devices.
None of these ioctls or fcntl()s is used often (except the TIOC* ones
for ttys). Perhaps the TIOC* ones have never been abused (to give
FIO*/fcntl() ones) for pipes.
>>> Be very very careful with this. =A0It's part of the classic BSD job
>>> control API. =A0It would be wise to survey whether any ports shells use
>>> this.
Well, not really. TIOC[SG]PGRP never gave job control for pipes.
>>> You might also want to consider things like this in libc:
>>> int
>>> tcsetpgrp(int fd, pid_t pgrp)
>>> {
>>> =A0 =A0 =A0 =A0int s;
>>>
>>> =A0 =A0 =A0 =A0s =3D pgrp;
>>> =A0 =A0 =A0 =A0return (_ioctl(fd, TIOCSPGRP, &s));
>>> }
>>> Our own libc code uses this, albeit on an API intended to be used on a =
tty.
This stuff in sys_pipe.c is simple compared with tty.c :-). tty.c actually
understands controlling terminals. In particular, it checks all the POSIX
conditions stated above for tcsetprgrp(). Its handling of FIO[GS]ETOWN is
quite different from its handling of TIOC[GS]PGRP. There are fewer
restrictions for the former, and the complications are in the f[sg]etown()
functions.
>>> The shell I'd be most concerned about is csh/tcsh in our tree. It has
>>> quite an #ifdef legacy layer and I couldn't convince myself it wasn't
>>> using this indirectly (or the tc* functions) on pipes.
>>>
>>> It might also be an idea to see if the linux compat layer can be
>>> switched over to using the newer API.
>>
>> Move to a compat library perhaps?
>> -Garrett
>
> It's not a compat library candidate.
>
> Summary of the issue:
> ioctl(fd, TIOCSPGRP, arg) can be used on sockets, pipes, ttys. But not f=
ifos.
> this is a classic BSD job control API.
Not really on pipes -- see above. Even less on sockets:
- sockets don't seem to ever have abused TIOC[SG]PGRP
- instead, for sockets, there is SIOC[SG]PGRP. This is used in sys_socket.=
c
in exactly the same way that TIOC[SG]PGRP is abused in sys_pipe.c.
- since its name is spelled with an 'S' (strictly, since its number is
spelled with an 's'), job control code like tcstpgrp() can't reach
the socket layer even if it is misapplied to a socket fd, and so it
can't bogusly succeed.
- SIOC[SG]PGRP shouldn't exist, since like TIOC[SG]PGRP on pipes, it
just reverses the sign, so it was superseded by FIO[SG]ETOWN (or
better the fcntl()). As for pipes, it now bogusly works on non-pgrps.
- SIOC[SG]PGRP is not documented in any man page. This matches its
supersedence, but was probably a bug -- it is a very old interface;
FreeBSD-1 has it for sockets.
(FreeBSD-1 also has TIOC*PGRP for the log device and perhaps for
some other devices outside of kern, but not for pipes). It also
has the following messy translations between FIO* and TIOC*, which
must have been cleaned up by truckman, and show that the old
way was probably not for these TIOC and FIO ioctls to actually
be used; instead, what always sort of worked was the fcntl(),
using these messy translations to get to older internals using
TIOC or FIO:
=09FreeBSD-1 ioctl():
% =09case FIOSETOWN:
% =09=09tmp =3D *(int *)data;
% =09=09if (fp->f_type =3D=3D DTYPE_SOCKET) {
% =09=09=09((struct socket *)fp->f_data)->so_pgid =3D tmp;
=09This starts with the FIOSETOWN ioctl. (Unfortunately, that is
=09a very old ioctl, so we can't just remove it.) This is
=09apparently not supported by socket code (it wants SIOCSPGRP?),
=09so hack up something here.
% =09=09=09error =3D 0;
% =09=09=09break;
% =09=09}
% =09=09if (tmp <=3D 0) {
% =09=09=09tmp =3D -tmp;
% =09=09} else {
% =09=09=09struct proc *p1 =3D pfind(tmp);
% =09=09=09if (p1 =3D=3D 0) {
% =09=09=09=09error =3D ESRCH;
% =09=09=09=09break;
% =09=09=09}
% =09=09=09tmp =3D p1->p_pgrp->pg_id;
% =09=09}
% =09=09error =3D (*fp->f_ops->fo_ioctl)
% =09=09=09(fp, (int)TIOCSPGRP, (caddr_t)&tmp, p);
=09For ttys but not much else, TIOCSPGRP exists and sort of
=09corresponds to FIOSETOWN. There is some overlap, but
=09especially for ttys, TIOCSPGRP has very different semantics,
=09so this call caused problems if anyone actually makes it.
=09It is likely to just fail, but I added workarounds to problems
=09near here in some versions of FreeBSD. These have been
=09replaced by better methods. E.g., pass FIOSETOWN to lower
=09levels here (done by truckman?).
% =09=09break;
=09FreeBSD-1 fcntl():
% =09case F_SETOWN:
% =09=09if (fp->f_type =3D=3D DTYPE_SOCKET) {
% =09=09=09((struct socket *)fp->f_data)->so_pgid =3D uap->arg;
% =09=09=09return (0);
% =09=09}
% =09=09if (uap->arg <=3D 0) {
% =09=09=09uap->arg =3D -uap->arg;
% =09=09} else {
% =09=09=09struct proc *p1 =3D pfind(uap->arg);
% =09=09=09if (p1 =3D=3D 0)
% =09=09=09=09return (ESRCH);
% =09=09=09uap->arg =3D p1->p_pgrp->pg_id;
% =09=09}
% =09=09return ((*fp->f_ops->fo_ioctl)
% =09=09=09(fp, (int)TIOCSPGRP, (caddr_t)&uap->arg, p));
=09Essentially the same as for the ioctl. The ioctl() is essentially
=09an archaic spelling of the fcntl(). This must be supported, but
=09hopefully its misimplementation details (mainly the translation
=09to a PGRP ioctl) don't need to be.
) <End FreeBSD-1 stuff>
<End SIOC[GS]PGRP stuff>
- the garbage or implementaion-detail TIOC[SG]PGRP ioctls are unfortunately
documented in many places:
- tap(4), tun(4), tty(4) (legit there), mdoc(7) (bad example)
and is used in the correspoding source places and others (now checking
more than kern but not all of sys):
- kern/subr_log.c, net/bpf.c: like sys/pipe.c
- net/if_tap.c, net/if_tun.c: like sys/pipe.c, except the ioctl is
actually documented.
> libc itself uses this internally on ttys.
> libc doesn't check that the function that does this is actually a tty,
> it could be being used by shells and pipes as an obscure side effect.
POSIX requires tcsetpgrp() etc to restrict to more than a tty (requires
controlling terminal and more), so libc must check if the kernel doesn't,
but this is very hard to do in libc.
> The API is equivalent to FIOSETOWN etc and sockets and pipes use
> common code to implement both ioctl() calls.
Actually, very different. Even F_SETOWN is merely similar to FIOSETOWN,
since the former operates on open files while the latter operates on
devices, at least logically.
> The proposed patch was to remove the TIOCSPGRP -> FIOSETOWN mapping
> for pipes only, to sync them with fifos. ttys and sockets would be
> unaffected.
No reason to remove it for pipes but not for logs, bpfs, taps or tuns.
Or break POSIX consistently and make the exceptional cases match pipes:
- fifos are missing support for TIOC[SG]PGRP in 2 ways:
- it is not passed to the socket layer
- the socket layer doesn't support it anyway, unless it is translated
to the socket layer spelling
- for sockets, it is spelled SIOC[SG]PGRP and undocumented.
Does POSIX specify sockets yet? If so it should disallow tiocsetpgrp()
on them, and give more details about how FSETOWN works on them.
> The implementation function still has to stay. Its not a large chunk
> of obsolete code that goes away, its just the mapping between the
> ioctl number and the implementation.
And it has nothing to do with what is deprecated :-).
Bruce
--0-1274663310-1321632703=:5050--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111119003059.V5050>