From: Yewon Choi <woni9911@gmail.com>
To: "Starke, Daniel" <daniel.starke@siemens.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jirislaby@kernel.org>,
linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
"Dae R. Jeong" <threeearcat@gmail.com>,
syzbot+6e3e8f30f269f5028e5d@syzkaller.appspotmail.com
Subject: Re: tty: n_gsm: race condition in gsmld_ioctl
Date: Wed, 17 Apr 2024 19:19:48 +0900 [thread overview]
Message-ID: <Zh+iROsYkqoRsJQj@libra05> (raw)
In-Reply-To: <DB9PR10MB5881D2170678C169FB42A423E0082@DB9PR10MB5881.EURPRD10.PROD.OUTLOOK.COM>
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
prev parent reply other threads:[~2024-04-17 10:19 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-16 10:57 tty: n_gsm: race condition in gsmld_ioctl Yewon Choi
2024-04-16 10:59 ` Jiri Slaby
2024-04-16 12:26 ` Starke, Daniel
2024-04-17 10:19 ` Yewon Choi [this message]
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=Zh+iROsYkqoRsJQj@libra05 \
--to=woni9911@gmail.com \
--cc=daniel.starke@siemens.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).