All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Weiming Shi" <bestswngs@gmail.com>
To: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Weiming Shi" <bestswngs@gmail.com>
Cc: "Jiri Slaby" <jirislaby@kernel.org>,
	"Shuah Khan" <shuah@kernel.org>,
	"Starke, Daniel" <daniel.starke@siemens.com>,
	"Xiang Mei" <xmei5@asu.edu>, <linux-serial@vger.kernel.org>,
	<linux-kselftest@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 0/2] tty: n_gsm: fix gsm_queue() UAF and add a base regression test
Date: Wed, 17 Jun 2026 13:54:25 +0800	[thread overview]
Message-ID: <DJB3B9GN52TI.P6PBYUEDRQW2@gmail.com> (raw)
In-Reply-To: <2026061722-explode-predator-59f4@gregkh>

On Wed Jun 17, 2026 at 9:24 AM CST, Greg Kroah-Hartman wrote:
> On Tue, Jun 16, 2026 at 10:32:38AM -0700, Weiming Shi wrote:
>> The receive worker walks gsm->dlci[] without gsm->mutex while a
>> concurrent GSMIOC_SETCONF -> gsm_cleanup_mux() frees the DLCIs, so the
>> control handlers can dereference a freed gsm_dlci. v1's NULL check only
>> narrowed the window; v2 fixes the use-after-free itself.
>> 
>> The fix pins each DLCI the dispatch dereferences with its existing
>> tty_port reference (option 2), so the data path stays lock-free. See the
>> patch 1 commit message for details, including why the late destructor
>> uses cmpxchg() so it cannot wipe a re-created mux (Daniel's teardown
>> concern).
>
> Cool, but wow, that's complex for something that will never actually
> happen in a real device :)
>
> So do we want to add that complexity?  if so, why?
>
> Ideally Daniel can verfiy this change is ok as they are the only known
> user here.
>
> And thanks for the test patch, but that's just a functional test, while
> great to have, and not one that can actually mimic a real device with
> its timing constraints, right?
>
> thanks,
>
> greg k-h

Hi,

The complexity is just the cmpxchg() in gsm_dlci_free(). The actual UAF fix
is only the tty_port pin (dlci_get/put) around the dispatch, which doesn't 
touch any fast path. The cmpxchg() only guards the teardown+recreate case 
Daniel mentioned, and isn't needed for the UAF.

So should I drop it and respin a v3 with the pin only? The use-after-free 
is fully fixed either way.

Daniel, does the pin approach look right to you?

And yes, the test is functional only (pty bring-up/teardown, no timing). 
It's the base regression test, not a race reproducer.

Thanks,
Weiming Shi


      reply	other threads:[~2026-06-17  5:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-16 17:32 [PATCH v2 0/2] tty: n_gsm: fix gsm_queue() UAF and add a base regression test Weiming Shi
2026-06-16 17:32 ` [PATCH v2 1/2] tty: n_gsm: fix use-after-free in gsm_queue() control frame dispatch Weiming Shi
2026-06-16 17:32 ` [PATCH v2 2/2] selftests: tty: add base regression test for n_gsm line discipline Weiming Shi
2026-06-17  1:24 ` [PATCH v2 0/2] tty: n_gsm: fix gsm_queue() UAF and add a base regression test Greg Kroah-Hartman
2026-06-17  5:54   ` Weiming Shi [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=DJB3B9GN52TI.P6PBYUEDRQW2@gmail.com \
    --to=bestswngs@gmail.com \
    --cc=daniel.starke@siemens.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=shuah@kernel.org \
    --cc=xmei5@asu.edu \
    /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.