Messages in this thread | | | From | Arnd Bergmann <> | Date | Thu, 13 Sep 2018 13:19:42 +0200 | Subject | Re: [PATCHES] tty ioctls cleanups, compat and not only |
| |
On Thu, Sep 13, 2018 at 4:31 AM Al Viro <viro@zeniv.linux.org.uk> wrote: > > See vfs.git#work.tty-ioctl. Completely untested, should seriously > clean the things up wrt compat. Remaining problems (aside of the bugs > introduced in it, of course): > * TIOCSERGSTRUCT must die; it's present only in amiserial and it's > _vile_; look at what it copies out and weep. > * synclink_gt has proper compat handling for its private ioctls; > other synclink drivers (with the same ioctls) do not. > * dgnc definitely has buggered ioctls - structs full of longs are > bloody bad idea for passing around. It's in staging, and I'd say that it > needs the userland ABI fixed. > * cyclades, rocket, moxa and mxser probably have non-trivial > problems with their private ioctls; I hadn't looked into those. > * n_gsm needs ->compat_ioctl(); easy to do, I just hadn't done it > yet. > * ipwireless might or might not need compat_ioctl (PPP stuff in it); > not sure. > * ldisc private ioctls need more work. Hadn't gone there yet. > Generic ioctls should be fine - they never reach ->compat_ioctl() with this > series.
I looked at all the new patches, looks all good to me. When I did my (incomplete) analysis, I found a couple more things that could be included in the series if we want to:
* TIOCSERGWILD/TIOCSERSWILD are obsolete and never do anything useful, but the return code is inconsistent: ENOTTY in compat mode vs 0 in uart_ioctl, plus a printk for amiserial. If we want to keep it around rather than deleting it completely, it should be marked as compatible.
* PPPIOCGCHAN/PPPIOCGUNIT are implemented by multiple ldisc variants, marking them as compatible would save us from implementing a comp_ioctl method for each one separately. These are also used on some things that are not ttys though, so we can't remove them from fs/compat_ioctl.c yet.
* FIONREAD/TIOCINQ and TIOCOUTQ also appear multiple times and could be added to that list but not removed from fs/compat_ioctl.c (I think the other users are all in sockets, so adding them in both might be sufficient)
* SIOCGIFNAME, SIOCGIFENCAP, SIOCSIFENCAP, SIOCSIFHWADDR, SIOCSKEEPALIVE, SIOCGKEEPALIVE, SIOCSOUTFILL, and SIOCGOUTFILL are in the tty_ioctl functions for multiple protocol handlers, comparable to the PPP ones.
> There's some overlap with patches posted earlier - e.g. I'm fairly > sure that keyboard ioctl "remove from fs/compat_ioctl.c" part duplicates > one of yours; I'll be glad to drop and replace such duplicates with already > posted variants.
Ok, don't worry about the order if you decide to apply yours first, you can just drop mine.
Arnd
| |