All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.