All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Lee Jones <lee@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	Daniel Starke <daniel.starke@siemens.com>,
	Fedor Pchelkin <pchelkin@ispras.ru>,
	Jiri Slaby <jirislaby@kernel.org>,
	linux-serial@vger.kernel.org,
	syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.com
Subject: Re: [PATCH 1/1] tty: n_gsm: Avoid sleeping during .write() whilst atomic
Date: Wed, 4 Oct 2023 14:19:29 +0200	[thread overview]
Message-ID: <2023100425-unwieldy-reaffirm-2a1b@gregkh> (raw)
In-Reply-To: <20231004090918.GB9374@google.com>

On Wed, Oct 04, 2023 at 10:09:18AM +0100, Lee Jones wrote:
> On Wed, 04 Oct 2023, Greg Kroah-Hartman wrote:
> 
> > On Tue, Oct 03, 2023 at 07:55:00PM +0100, Lee Jones wrote:
> > > On Tue, 03 Oct 2023, Greg Kroah-Hartman wrote:
> > > 
> > > > On Tue, Oct 03, 2023 at 06:00:20PM +0100, Lee Jones wrote:
> > > > > The important part of the call stack being:
> > > > > 
> > > > >   gsmld_write()             # Takes a lock and disables IRQs
> > > > >     con_write()
> > > > >       console_lock()
> > > > 
> > > > Wait, why is the n_gsm line discipline being used for a console?
> > > > 
> > > > What hardware/protocol wants this to happen?
> > > > 
> > > > gsm I thought was for a very specific type of device, not a console.
> > > > 
> > > > As per:
> > > > 	https://www.kernel.org/doc/html/v5.9/driver-api/serial/n_gsm.html
> > > > this is a specific modem protocol, why is con_write() being called?
> > > 
> > > What it's meant for and what random users can make it do are likely to
> > > be quite separate questions.  This scenario is user driven and can be
> > > replicated simply by issuing a few syscalls (open, ioctl, write).
> > 
> > I would recommend that any distro/system that does not want to support
> > this specific hardware protocol, just disable it for now (it's marked as
> > experimental too), if they don't want to deal with the potential
> > sleep-while-atomic issue.
> 
> n_gsm is available on all the systems I have available.  The mention of
> 'EXPERIMENTAL' in the module description appears to have zero effect on
> whether distros choose to make it available or not.  If you're saying
> that we know this module is BROKEN however, then perhaps we should mark
> it as such.

Also, I think this requires root to set this line discipline to the
console, right?  A normal user can't do that, or am I missing a code
path here?

Is there a reproducer somewhere for this issue that runs as a normal
user?  I couldn't find one in the syzbot listings but I might have been
not looking deep enough.

thanks,

greg k-h

  parent reply	other threads:[~2023-10-04 12:19 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-03 17:00 [PATCH 1/1] tty: n_gsm: Avoid sleeping during .write() whilst atomic Lee Jones
2023-10-03 18:14 ` Greg Kroah-Hartman
2023-10-03 18:55   ` Lee Jones
2023-10-04  6:04     ` Greg Kroah-Hartman
2023-10-04  9:09       ` Lee Jones
2023-10-04  9:55         ` Greg Kroah-Hartman
2023-10-04 12:19         ` Greg Kroah-Hartman [this message]
2023-10-04 12:57           ` Lee Jones
2023-10-04 13:13             ` Greg Kroah-Hartman
2023-10-05  4:59             ` Jiri Slaby
2023-10-05  6:05               ` Greg Kroah-Hartman
2023-10-05  7:26                 ` Lee Jones
2023-10-04  5:59   ` Starke, Daniel
2023-10-04  6:05     ` Greg Kroah-Hartman
2023-10-04  8:40       ` Jiri Slaby
2023-10-04  8:58         ` Starke, Daniel
2023-10-04  8:57       ` Lee Jones
2023-10-04 12:21         ` Greg Kroah-Hartman
2023-10-04 12:57           ` Lee Jones
2023-10-04 13:14             ` Greg Kroah-Hartman
2023-10-05  9:03               ` Lee Jones
2023-10-05  9:18                 ` Greg Kroah-Hartman
2023-10-05 10:43                   ` Lee Jones
2023-10-05 11:33                     ` Greg Kroah-Hartman
2023-10-05 11:38                       ` Starke, Daniel
2023-10-05 11:46                         ` Lee Jones
2023-10-05 11:55                           ` Greg Kroah-Hartman
2023-10-05 12:17                             ` Lee Jones
2023-10-05 12:37                               ` Greg Kroah-Hartman

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=2023100425-unwieldy-reaffirm-2a1b@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=daniel.starke@siemens.com \
    --cc=jirislaby@kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=pchelkin@ispras.ru \
    --cc=syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.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.