From:   "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To:     elver@google.com, gregkh@linuxfoundation.org, jirislaby@kernel.org,
        linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
        syzbot <syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.com>
Subject: Re: [syzbot] BUG: sleeping function called from invalid context in __might_resched
Date:   Tue, 16 Nov 2021 10:03:12 +0100
Message-ID: <7655311.pJQ0NNsvTN@localhost.localdomain>
In-Reply-To: <5631140.hc6UrLtR2d@localhost.localdomain>
References: <000000000000be6cff05d0e3709b@google.com> <5631140.hc6UrLtR2d@localhost.localdomain>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="nextPart2046742.yJbYBYnt3e"
Content-Transfer-Encoding: 7Bit
Precedence: bulk

This is a multi-part message in MIME format.

--nextPart2046742.yJbYBYnt3e
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="ISO-8859-1"

On Tuesday, November 16, 2021 9:53:53 AM CET Fabio M. De Francesco wrote:
> On Tuesday, November 16, 2021 9:09:11 AM CET syzbot wrote:
> > Hello,
> > 
> > syzbot has tested the proposed patch but the reproducer is still 
triggering 
> an issue:
> > BUG: sleeping function called from invalid context in __might_resched
> > [...]
> 
> The reproducer is still triggering an issue, but this time it looks like it 
> is triggered by a different path of execution.
> 
> The same invalid "in_interrupt()" test is also in con_flush_chars().
> 
> Let's try to remove it too...
> 
> My first idea would be to replace "if (in_interrupt())" with the same 
> "preempt_count() || irqs_disabled()" I used in do_con_write(). However I 
> noticed that both do_con_write() and con_flush_chars() are only called from 
> inside con_write() (which, aside from calling those functions, does nothing 
> else).
> 
> So why not remove the if (in_interrupt()) from both them and use if 
> (preempt_count() || irqs_disabled()) just once in con_write()?
> 
> I think this should be the right solution, but I prefer to go one step at a 
> time.
> 
> Therefore, I'll (1) use the same (redundant, if it was used in con_write()) 
> test also in con_flush_chars(), (2) wait for Syzbot to confirm that it 
fixes 
> the bug, and (3) wait for maintainers review and suggestions about whether 
or 
> not moving those tests one level upper.
> 
> #syz test:
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> 
> ---
> Fabio M. De Francesco

Don't know exactly what happened, so let's retry the test...

#syz test: 
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

--nextPart2046742.yJbYBYnt3e
Content-Disposition: attachment; filename="vt.c.diff"
Content-Transfer-Encoding: 7Bit
Content-Type: text/x-patch; charset="UTF-8"; name="vt.c.diff"

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 7359c3e80d63..46511d1ac6ee 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 (preempt_count() || irqs_disabled())
 		return count;
 
 	console_lock();
@@ -3358,7 +3358,7 @@ static void con_flush_chars(struct tty_struct *tty)
 {
 	struct vc_data *vc;
 
-	if (in_interrupt())	/* from flush_to_ldisc */
+	if (preempt_count() || irqs_disabled())	/* from flush_to_ldisc */
 		return;
 
 	/* if we race with con_close(), vt may be null */

--nextPart2046742.yJbYBYnt3e--