On Tue, Apr 16, 2024 at 12:26:30PM +0000, Starke, Daniel wrote: > > We think either (1) gsm_dlci_alloc() should hold a lock(mutex) and do > > internal check about whether gsm->dlci[addr] is NUll or not, OR > > (2) all callers of gsm_dlci_alloc() should hold gsm->mutex and check > > whether gsm->dlci[addr] is NUll or not (like gsmtty_install()). > > > > Could you check this? If it makes sense, we will write a patch > > following one of the suggestions. > > We are currently dealing with multiple race conditions in the n_gsm. Not > only for the syzkaller reports but in recent exploits for example, too. > I believe we need an overall concept/solution for these. > > Currently, the following actors can race: > - ioctl (e.g. resetting the mux or one of its DLCIs) > - ldisc callbacks (e.g. receiving SABM or DISC in gsm_queue()) > - tty callbacks (e.g. gsmtty_hangup()) > - internal write task (gsmld_write_task()) > - internal timers (e.g. gsm_control_keep_alive()) > - driver removal > > Each with another and ioctl's from multiple threads. > > Guarding these is not trivial for the following reasons: > - execution context may not allow sleep (timers, write task, tty callbacks?) > - creating an atomic section may conflict inner sleeps (e.g. ioctl) > - dead-locking needs to be considered > - locking may introduce high performance bottlenecks > > Still, not guarding one of the racing actors does not appears to be > possible as I see it. > > Unfortunately, for the same reason the sleep while atomic issue when using > a console on a virtual tty has not been fixed yet, I have no solution at > hand. We are dealing with a quite complex situation here. > > Nevertheless, creating many small patches here and there should not be our > solution for obvious reasons. This does not give a complete solution and > makes it harder to find the remaining corner cases. > > I can assist to find a solution here, but I have not enough time to do this > alone at the moment. > Thank you for your comprehensive explanation. As you described, there are many concurrency issues entangled each other and fixing single one will be rather confusing. I hope this mail is supportive on solving the problems. Sincerely, Yewon Choi > Best regards, > Daniel Starke
