MIME-Version: 1.0
Date:   Tue, 16 Nov 2021 01:03:15 -0800
In-Reply-To: <7655311.pJQ0NNsvTN@localhost.localdomain>
Message-ID: <00000000000022f93a05d0e432f3@google.com>
Subject: Re: [syzbot] BUG: sleeping function called from invalid context in __might_resched
From:   syzbot <syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.com>
To:     "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
Cc:     elver@google.com, fmdefrancesco@gmail.com,
        gregkh@linuxfoundation.org, jirislaby@kernel.org,
        linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

> 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: 

want 2 args (repo, branch), got 1

> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git