RE: [PATCH] tty: n_gsm: fix NULL deref of gsm->dlci[0] in control message handlers

[Starke, Daniel]

> > I think the fix should serialize the receive side against
> > gsm_cleanup_mux() instead of checking in the handlers. Two ways I can see:
> > 
> >   1. take gsm->mutex around the dlci lookup and dispatch in gsm_queue(), or
> >   2. pin the dlci across the dispatch using its existing tty_port ref
> >     (dlci_get/dlci_put), so gsm_dlci_free() can't run while it's in use.
> > 
> > Do you have a preference, or is there a pattern in n_gsm you'd rather 
> > I use? I'll respin v2 once I know which way to go.
> 
> The bigger issue here is that almost no one has this hardware.  And those that do, don't care about these types of issues as they do not have untrusted data or untrusted users, so be careful when changing things that you aren't able to test.
> 
> I think that option 2 would probably be best, as that should not affect any fast code paths, right?

A side note from my side: Using mutex instead of spinlock within the data
transmission path has been tested in the past with a huge performance
regression as a result. This made the protocol unreliable in production due
to its tight timing requirements. Hence, I also vote for option 2. I just
wonder how you plan to implement this. The racing GSMIOC_SETCONF ioctl,
which triggers the gsm_cleanup_mux(), does so in order to re-create the
virtual ttys. It can only do so by a full teardown. If you prevent the
teardown there, the new devices will fail to register in the next step or
the interface will be left with an unpredictable configuration.

See related mail from the past:
https://lore.kernel.org/linux-serial/Zh+iROsYkqoRsJQj@libra05/

Best regards,
Daniel Starke