Hello & thanks. To: Daniel On 16. 04. 24, 12:57, Yewon Choi wrote:
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
-- js
