From: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>,
Max Filippov <jcmvbkbc@gmail.com>,
David Sterba <dsterba@suse.com>,
Bhaskar Chowdhury <unixbhaskar@gmail.com>,
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
Igor Matheus Andrade Torrente <igormtorrente@gmail.com>,
nick black <dankamongmen@gmail.com>,
linux-kernel@vger.kernel.org,
syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.com,
Marco Elver <elver@google.com>
Subject: Re: [PATCH] vt: Fix sleeping functions called from atomic context
Date: Tue, 16 Nov 2021 16:35:07 +0100 [thread overview]
Message-ID: <2524108.PJBYKFOWIp@localhost.localdomain> (raw)
In-Reply-To: <YZPHJE2R4VCQ20Za@kroah.com>
On Tuesday, November 16, 2021 3:58:44 PM CET Greg Kroah-Hartman wrote:
> On Tue, Nov 16, 2021 at 03:49:37PM +0100, Fabio M. De Francesco wrote:
> > Fix two sleeping functions called from atomic context by doing immediate
> > return to the caller if !preemptible() evaluates 'true'. Remove two
> > in_interrupt() tests because they are not suited for being used here.
> >
> > Since functions do_con_write() and con_flush_chars() might sleep in
> > console_lock(), it must be assured that they are never executed in
> > atomic contexts.
> >
> > This issue is reported by Syzbot which notices that they are executed
> > while holding spinlocks and with interrupts disabled. Actually Syzbot
> > emits a first report and then, after fixing do_con_write(), a second
> > report for the same problem in con_flush_chars() because these functions
> > are called one after the other by con_write().
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Reported-by: syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.com
> > Suggested-by: Marco Elver <elver@google.com>
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> > drivers/tty/vt/vt.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> > index 7359c3e80d63..508f8a56d361 100644
> > --- a/drivers/tty/vt/vt.c
> > +++ b/drivers/tty/vt/vt.c
> > @@ -2902,7 +2902,7 @@ static int do_con_write(struct tty_struct *tty,
const unsigned char *buf, int co
> > struct vt_notifier_param param;
> > bool rescan;
> >
> > - if (in_interrupt())
> > + if (!preemptible())
> > return count;
>
> Very odd, what code is calling these functions to trigger this check?
This is the call trace reported by Syzbot (https://syzkaller.appspot.com/bug?
id=fe5a4d5a2482bd73064db5de5d28e024f1e2a387):
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
__might_resched.cold+0x222/0x26b kernel/sched/core.c:9539
console_lock+0x17/0x80 kernel/printk/printk.c:2522
do_con_write+0x10f/0x1e40 drivers/tty/vt/vt.c:2908
con_write+0x21/0x40 drivers/tty/vt/vt.c:3295
n_hdlc_send_frames+0x24b/0x490 drivers/tty/n_hdlc.c:290
tty_wakeup+0xe1/0x120 drivers/tty/tty_io.c:534
__start_tty drivers/tty/tty_io.c:806 [inline]
__start_tty+0xfb/0x130 drivers/tty/tty_io.c:799
n_tty_ioctl_helper+0x299/0x2d0 drivers/tty/tty_ioctl.c:880
^^^^^^^^^^
n_tty_ioctl_helper() disabled interrupts via spin_lock_irq(&tty->flow.lock).
n_hdlc_tty_ioctl+0xd2/0x340 drivers/tty/n_hdlc.c:633
tty_ioctl+0xc69/0x1670 drivers/tty/tty_io.c:2814
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:874 [inline]
__se_sys_ioctl fs/ioctl.c:860 [inline]
__x64_sys_ioctl+0x193/0x200 fs/ioctl.c:860
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
> Shouldn't the caller be fixed instead?
Maybe that the caller has no need to disable IRQs, but I cannot yet answer to
this particular question.
> What changed to suddenly cause this to show up?
Commit c545b66c6922 ("tty: Serialize tcflow() with other tty flow control
changes") introduced a call to spin_lock_irq() for command "TCOON", just
before calling __start_tty().
Thanks,
Fabio M. De Francesco
> Given that this check has been here for a _very_ long time, changing it
> now without finding out the root cause is probably not a good idea.
>
> thanks,
>
> greg k-h
>
next prev parent reply other threads:[~2021-11-16 15:35 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-16 14:49 [PATCH] vt: Fix sleeping functions called from atomic context Fabio M. De Francesco
2021-11-16 14:58 ` Greg Kroah-Hartman
2021-11-16 15:35 ` Fabio M. De Francesco [this message]
2021-11-16 16:59 ` Greg Kroah-Hartman
2021-11-16 17:28 ` Fabio M. De Francesco
2021-11-17 8:23 ` Fabio M. De Francesco
2021-11-17 8:54 ` Greg Kroah-Hartman
2021-11-17 10:51 ` Tetsuo Handa
2021-11-18 8:31 ` Fabio M. De Francesco
2021-11-18 9:38 ` Fabio M. De Francesco
2021-11-18 12:14 ` Tetsuo Handa
2021-11-18 17:01 ` Fabio M. De Francesco
2021-11-19 14:55 ` [PATCH] tty: vt: make do_con_write() no-op if IRQ is disabled Tetsuo Handa
2021-12-01 13:40 ` Tetsuo Handa
2021-12-01 14:20 ` Greg Kroah-Hartman
2021-12-01 19:05 ` Linus Torvalds
2021-12-02 15:40 ` Tetsuo Handa
2021-12-02 18:35 ` Linus Torvalds
2021-12-03 5:03 ` Jiri Slaby
2021-12-03 11:00 ` Fabio M. De Francesco
2021-12-03 12:32 ` Tetsuo Handa
2021-12-03 14:51 ` Fabio M. De Francesco
2021-11-17 12:38 ` [PATCH] vt: Fix sleeping functions called from atomic context Fabio M. De Francesco
2021-11-17 1:55 ` Tetsuo Handa
2021-11-17 7:02 ` Fabio M. De Francesco
2021-12-06 11:44 ` [PATCH] tty: n_hdlc: make n_hdlc_tty_wakeup() asynchronous Tetsuo Handa
2021-12-06 18:07 ` Linus Torvalds
2021-12-09 13:18 ` Tetsuo Handa
2021-12-15 11:52 ` [PATCH (resend)] " Tetsuo Handa
2021-12-06 19:06 ` [PATCH] " Fabio M. De Francesco
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=2524108.PJBYKFOWIp@localhost.localdomain \
--to=fmdefrancesco@gmail.com \
--cc=dankamongmen@gmail.com \
--cc=dsterba@suse.com \
--cc=elver@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=igormtorrente@gmail.com \
--cc=jcmvbkbc@gmail.com \
--cc=jirislaby@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.com \
--cc=unixbhaskar@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.