Re: [PATCH v2] tty: n_gsm: restrict tty devices to attach

[Tetsuo Handa]

On 2024/04/22 1:04, Linus Torvalds wrote:
>> Now, your 'struct tty_operations' flag saying 'my ->write() function is OK with
>> atomic context' is expected to be set to all drivers.
> 
> I'm not convinced. The only thing I know is that the comment in
> question is wrong, and has been wrong for over a decade (and honestly,
> probably pretty much forever).
> 
> So how confident are we that other tty write functions are ok?
> 
> Also, since you think that only con_write() has a problem, why the
> heck are you then testing for ptys etc? From a quick check, the
> pty->ops->write() function is fine.

I tried to make deny-listing as close as allow-listing. But it seems that
ipw_write() in drivers/tty/ipwireless/tty.c (e.g. /dev/ttyIPWp0 ) does
sleep as well as con_write() in drivers/tty/vt/vt.c .

I couldn't judge serial_write() in drivers/usb/serial/usb-serial.c (e.g.
/dev/ttyUSB0 ). But since device number for /dev/ttyIPWp0 is assigned
dynamically, we can't rely on major/minor for detecting /dev/ttyIPWp0 from
gsmld_open() side.

That is, we need to handle this problem from "struct tty_operations" side
(like I initially tried).



On 2024/04/22 2:18, Linus Torvalds wrote:
> On Sun, 21 Apr 2024 at 09:04, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>>
>> The only option is to *mark* the ones that are atomic. Which was my suggestion.

Since majority of "struct tty_operations"->write() seems to be atomic,
I prefer updating only ones which cannot be atomic.

> 
> Actually, another option would be to just return an error at 'set_ldisc()' time.
> 
> Sadly, the actual "tty->ops->set_ldisc()" function not only returns
> 'void' (easy enough to change - there aren't that many of them), but
> it's called too late after the old ldisc has already been dropped.
> It's basically a "inform tty about new ldisc" and is not useful for a
> "is this ok"?
> 
> But we could trivially add a "ldisc_ok()" function, and have the vt
> driver say "I only accept N_TTY".
> 
> Something like this ENTIRELY UNTESTED patch.
> 
> Again - this is untested, and maybe there are other tty drivers that
> have issues with the stranger line disciplines, but this at least
> seems simple and fairly easy to explain why we do what we do..

This patch works for me. You can propose a formal patch.