From: Linus Torvalds <torvalds@linux-foundation.org>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jirislaby@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
"Starke, Daniel" <daniel.starke@siemens.com>,
LKML <linux-kernel@vger.kernel.org>,
linux-security-module <linux-security-module@vger.kernel.org>
Subject: Re: [PATCH v2] tty: n_gsm: restrict tty devices to attach
Date: Sun, 21 Apr 2024 09:04:31 -0700 [thread overview]
Message-ID: <CAHk-=wgjZ0DJgeo5Sk-Kc5vw8TXGuxXftPV79Wv221ncstk1tA@mail.gmail.com> (raw)
In-Reply-To: <6103a212-f84f-4dad-9d33-a18235bd970a@I-love.SAKURA.ne.jp>
On Sun, 21 Apr 2024 at 06:28, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> "struct tty_ldisc_ops" says that ->write() function (e.g. gsmld_write())
> is allowed to sleep and "struct tty_operations" says that ->write() function
> (e.g. con_write()) is not allowed to sleep.
Well, clearly con_write() *is* allowed to sleep. The very first thing
it does is that
console_lock();
thing, which uses a sleeping semaphore.
But yes, the comment in the header does say "may not sleep".
Clearly that comment doesn't actually reflect reality - and never did.
The console lock sleeping isn't some new thing (ie it doesn't come
from the somewhat recent printk changes).
So the comment is bogus and wrong.
> Thus, I initially proposed
> https://lkml.kernel.org/r/9cd9d3eb-418f-44cc-afcf-7283d51252d6@I-love.SAKURA.ne.jp
> which makes con_write() no-op when called with IRQs disabled.
The thing is, that's not the only thing that makes atomic context.
And some atomic contexts cannot be detected at run-time, they are
purely static (ie being inside a spinlock withg a !PREEMPT kernel
build).
So you cannot test for this.
The only option is to *mark* the ones that are atomic. Which was my suggestion.
> My major/minor approach is based on a suggestion from Jiri that we just somehow
> disallow attaching this line discipline to a console
Since we already know that the comment is garbage, why do you think
it's just a con_write() that has this issue?
And if it is only the console that has this issue, why are you testing
for other major/minor numbers?
> Now, your 'struct tty_operations' flag saying 'my ->write() function is OK with
> atomic context' is expected to be set to all drivers.
I'm not convinced. The only thing I know is that the comment in
question is wrong, and has been wrong for over a decade (and honestly,
probably pretty much forever).
So how confident are we that other tty write functions are ok?
Also, since you think that only con_write() has a problem, why the
heck are you then testing for ptys etc? From a quick check, the
pty->ops->write() function is fine.
Linus
next prev parent reply other threads:[~2024-04-21 16:04 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-20 11:12 [PATCH v2] tty: n_gsm: restrict tty devices to attach Tetsuo Handa
2024-04-20 13:13 ` Greg Kroah-Hartman
2024-04-20 17:34 ` Linus Torvalds
2024-04-20 18:02 ` Linus Torvalds
2024-04-20 18:05 ` Linus Torvalds
2024-04-21 13:28 ` Tetsuo Handa
2024-04-21 16:04 ` Linus Torvalds [this message]
2024-04-21 17:18 ` Linus Torvalds
2024-04-23 15:26 ` Tetsuo Handa
2024-04-23 16:37 ` Linus Torvalds
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='CAHk-=wgjZ0DJgeo5Sk-Kc5vw8TXGuxXftPV79Wv221ncstk1tA@mail.gmail.com' \
--to=torvalds@linux-foundation.org \
--cc=akpm@linux-foundation.org \
--cc=daniel.starke@siemens.com \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
/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.