public inbox for linux-serial@vger.kernel.org
 help / color / mirror / Atom feed
From: Yewon Choi <woni9911@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org
Cc: "Dae R. Jeong" <threeearcat@gmail.com>,
	syzbot+6e3e8f30f269f5028e5d@syzkaller.appspotmail.com
Subject: tty: n_gsm: race condition in gsmld_ioctl
Date: Tue, 16 Apr 2024 19:57:19 +0900	[thread overview]
Message-ID: <Zh5Zj35zeobGGzKj@libra05> (raw)

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

             reply	other threads:[~2024-04-16 10:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-16 10:57 Yewon Choi [this message]
2024-04-16 10:59 ` tty: n_gsm: race condition in gsmld_ioctl Jiri Slaby
2024-04-16 12:26   ` Starke, Daniel
2024-04-17 10:19     ` Yewon Choi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zh5Zj35zeobGGzKj@libra05 \
    --to=woni9911@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=syzbot+6e3e8f30f269f5028e5d@syzkaller.appspotmail.com \
    --cc=threeearcat@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox