tty: n_gsm: race condition in gsmld_ioctl

From: Yewon Choi
Date: Tue Apr 16 2024 - 06:57:31 EST


Hello,

memory leak in gsmld_ioctl was reported by syzbot, and this issue seems to be
raised from race condition involving gsm->dlci[addr].

https://syzkaller.appspot.com/bug?extid=6e3e8f30f269f5028e5d

When dlci object is checked and initialized in gsmld_ioctl(), it may be
assigned multiple times because there is no lock which guards entering
the critical section in gsm_dlci_alloc() from multiple threads.

For example, when multiple ioctl() are called concurrently, the following
scenario is possible:

Thread 0 Thread 1
ioctl(GSMIOC_SETCONF_DLCI) ioctl(GSMIOC_GETCONF_DLCI)

gsmld_ioctl(): gsmld_ioctl():
dlci = gsm->dlci[addr];
dlci = gsm->dlci[addr];
if (!dlci) {
gsm_dlci_alloc(gsm, addr):
...
gsm->dlci[addr] = dlci;
if (!dlci) {
gsm_dlci_alloc(gsm, addr):
...
// overwritten & memory leak
gsm->dlci[addr] = dlci;


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.

Best Regards,
Yewon Choi