Re: [syzbot] [sound?] BUG: sleeping function called from invalid context in snd_card_locked

[Takashi Iwai <tiwai@suse.de>]

On Sat, 01 Mar 2025 10:50:43 +0100,
Zhongqiu Han wrote:
> 
> On 3/1/2025 5:34 PM, Takashi Iwai wrote:
> > On Sat, 01 Mar 2025 10:25:55 +0100,
> > Zhongqiu Han wrote:
> >> 
> >>> Hello,
> >>> 
> >>> syzbot found the following issue on:
> >>> 
> >>> HEAD commit:    d082ecbc71e9 Linux 6.14-rc4
> >>> git tree:       upstream
> >>> console output: https://syzkaller.appspot.com/x/log.txt?x=14e3d7a4580000
> >>> kernel config:
> >> https://syzkaller.appspot.com/x/.config?x=8f2f8fb6ad08b539
> >>> dashboard link:
> >> https://syzkaller.appspot.com/bug?extid=4cb9fad083898f54c517
> >>> compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils
> >> for Debian) 2.40
> >> 
> >> 
> >> BUG: sleeping function called from invalid context and
> >> raw_local_irq_restore() called with IRQs enabled seems can be
> >> fixed by below change. if it is valid, will arise the PATCH.
> > 
> > snd_timer_process_callbacks() gets called from two places, one from
> > snd_timer_work() and another from snd_timer_interrupt() where both
> > caller cover already with guard(spinlock_irqsave).  That is, it's a
> > nested lock, hence without _irqsave().
> > 
> > IMO, the question is rather why the check of "!in_interrupt()" in
> > snd_seq_client_use_ptr() passed in this call path.
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> 
> Thanks Takashi for the discussion.
> 
> I have an initial check:
> func snd_seq_check_queue is called from func snd_seq_timer_interrupt,
> and the scoped_guard can not cover it. maybe this the reason of
> !in_interrupt() check pass.
> 
> just like my patch shared, snd_timer_process_callbacks called
> spin_unlock but not spin_unlock_irqrestore, which caused
> irqs_disabled(): 1 , and then caused the BUG.
> 
> 
> BUG: sleeping function called from invalid context at
> kernel/locking/mutex.c:562
> in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1167, name:
> kworker/0:1H
> 
> 
> please feel free and kindly correct me if any misunderstanding.

Ah, no, the code in timer.c worked as expected; the lock in the caller
side is temporarily released intentionally for avoiding deadlock.

It's rather the problem in seq_clientmgr.c side, as I mentioned.  The
check with !in_interrupt() is fragile in this case, and it's an
overkill to handle the module loading whenever it's referenced.

I'm going to submit the fix patch later.


thanks,

Takashi