linux-kernel - Re: [PATCH] tty: vt: make do_con_write() no-op if IRQ is disabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wiXNJ86W=gwAHH1qd+cE9dmfk_dEKFmNa89XH19NhPNkg@mail.gmail.com>
Date:   Thu, 2 Dec 2021 10:35:16 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Peter Hurley <peter@...leysoftware.com>,
        Jiri Slaby <jirislaby@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] tty: vt: make do_con_write() no-op if IRQ is disabled

On Thu, Dec 2, 2021 at 7:41 AM Tetsuo Handa
<penguin-kernel@...ove.sakura.ne.jp> wrote:
>
> > Looking at the backtrace, I see
> >
> >    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
> >
> > and apparently it's that hdlc line discipline (and
> > n_hdlc_send_frames() in particular) that is the problem here.
> >
> > I think that's where the fix should be.
>
> Do you mean that we should change the behavior of n_hdlc_send_frames()
> rather than trying to make __start_tty() schedulable again?

I wouldn't change n_hdlc_send_frames() itself. It does what it says it does.

But n_hdlc_tty_wakeup() probably shouldn't call it directly. Other tty
line disciplines don't do that kind of thing - although I only looked
at a couple. They all seem to just set bits and prepare things. Like a
wakeup function should do.

So I think n_hdlc_tty_wakeup() should perhaps only do a
"schedule_work()" or similar to get that n_hdlc_send_frames() started,
rather than doing it itself.

Example: net/nfc/nci/uart.c. It does that

        schedule_work(&nu->write_work);

instead of actually trying to do a write from a wakeup routine
(similar examples in ppp - "tasklet_schedule(&ap->tsk)" etc).

I mean, it's called "wakeup", not "write". So I think the fundamental
confusion here is in hdlc, not the tty layer.

              Linus