Re: [Intel-gfx] [PATCH] ALSA: hda: fix general protection fault in azx_runtime_idle

[Takashi Iwai <tiwai@suse.de>]

On Fri, 12 Nov 2021 13:27:34 +0100,
Kai Vehmanen wrote:
> 
> Hi,
> 
> On Fri, 12 Nov 2021, Takashi Iwai wrote:
> 
> > On Thu, 11 Nov 2021 18:39:36 +0100, Kai Vehmanen wrote:
> > > And later
> > > [   54.770701] Enabling runtime PM for inactive device (0000:00:1f.3) with active children
> > > [   54.770718] WARNING: CPU: 0 PID: 10 at drivers/base/power/runtime.c:1439 pm_runtime_enable+0x98/0xb0
> > > 
> > > Adding a "pm_runtime_set_active(&pci->dev)" to both azx_free() and 
> > > azx_probe_continue() seems to help and fix still works.
> > 
> > Ah yes, I was confused as if it were already called in hdac_device.c,
> > but this was about the HD-audio bus controller, not the codec.
> > 
> > Below is the revised one.
> [...]
> > Currently we haven't explicitly enable and allow/forbid the runtime PM
> > at the probe and the remove phases of HD-audio controller driver, and
> > this was the reason of a GPF mentioned in the commit e81478bbe7a1
> > ("ALSA: hda: fix general protection fault in azx_runtime_idle");
> > namely, even after the resources are released, the runtime PM might be
> > still invoked by the bound graphics driver during the remove of the
> > controller driver.  Although we've fixed it by clearing the drvdata
> > reference, it'd be also better to cover the runtime PM issue more
> > properly.
> > 
> > This patch adds a few more pm_runtime_*() calls at the probe and the
> > remove time for setting and cleaning up the runtime PM.  Particularly,
> > now more explicitly pm_runtime_enable() and _disable() get called as
> > well as pm_runtime_forbid() call at the remove callback, so that a
> > use-after-free should be avoided.
> > 
> > Reported-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> ack, tested this and no warnings anymore.
> 
> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> Tested-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>

OK, I'll submit and merge the patch.

As the original problem was fixed by NULL checks, I'd push this for
5.16, though.


thanks,

Takashi