Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230627095616.5333-1-dg573847474@gmail.com> <87jzvpfcli.wl-tiwai@suse.de>
In-Reply-To: <87jzvpfcli.wl-tiwai@suse.de>
From:   =?UTF-8?B?5Y+25r6E6ZSL?= <dg573847474@gmail.com>
Date:   Tue, 27 Jun 2023 18:55:19 +0800
Message-ID: <CAAo+4rV6qbUftFFDX0q1+tkbmZNrY7nN2b12B4GhsjJ64k6PVg@mail.gmail.com>
Subject: Re: [PATCH] ALSA: sb: fix potential deadlock on &chip->mixer_lock
To:     Takashi Iwai <tiwai@suse.de>
Cc:     perex@perex.cz, tiwai@suse.com, alsa-devel@alsa-project.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Oh yes, sorry for not considering that one.

Best regards
Chengfeng

Takashi Iwai <tiwai@suse.de> =E4=BA=8E2023=E5=B9=B46=E6=9C=8827=E6=97=A5=E5=
=91=A8=E4=BA=8C 18:01=E5=86=99=E9=81=93=EF=BC=9A
>
> On Tue, 27 Jun 2023 11:56:16 +0200,
> Chengfeng Ye wrote:
> >
> > As &chip->mixer_lock is also acquired by the irq snd_sb8dsp_interrupt()
> > which executes under hard-irq context, code executing under process
> > context should disable irq before acquiring the lock, otherwise
> > deadlock could happen if the process context hold the lock then
> > preempt by the interruption.
> >
> > As the ALSA Driver document described, PCM prepare callbacks are not
> > executed with irq disabled by default, thus the acquiring of
> > &chip->mixer_lock should be irq disabled.
> >
> > Possible deadlock scenario:
> > snd_sb8_playback_prepare
> >     -> spin_lock(&chip->mixer_lock);
> >         <irq interrupt>
> >         -> snd_sb8dsp_interrupt()
> >         -> snd_sb8_capture_trigger()
> >         -> spin_lock(&chip->mixer_lock); (deadlock here)
> >
> > This flaw was found using an experimental static analysis tool we are
> > developing for irq-related deadlock.
> >
> > The tentative patch fix the potential deadlock by spin_lock_irqsave().
> >
> > Signed-off-by: Chengfeng Ye <dg573847474@gmail.com>
>
> I believe it's a false-positive.  There is already a call
>         spin_lock_irqsave(&chip->reg_lock, flags);
> beforehand.
>
>
> thanks,
>
> Takashi