From: Takashi Iwai <tiwai@suse.de> To: "Deucher, Alexander" <Alexander.Deucher@amd.com> Cc: Lukas Wunner <lukas@wunner.de>, Jaroslav Kysela <perex@perex.cz>, Mika Westerberg <mika.westerberg@linux.intel.com>, Bjorn Helgaas <helgaas@kernel.org>, Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>, "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org> Subject: Re: [PATCH] ALSA: hda/hdmi - Fix duplicate unref of pci_dev Date: Tue, 10 Dec 2019 17:10:40 +0100 [thread overview] Message-ID: <s5h4ky8w48v.wl-tiwai@suse.de> (raw) In-Reply-To: <MWHPR12MB1358449C677259C848AAB11EF75B0@MWHPR12MB1358.namprd12.prod.outlook.com> On Tue, 10 Dec 2019 16:53:20 +0100, Deucher, Alexander wrote: > > > -----Original Message----- > > From: Lukas Wunner <lukas@wunner.de> > > Sent: Tuesday, December 10, 2019 10:47 AM > > To: Deucher, Alexander <Alexander.Deucher@amd.com> > > Cc: Takashi Iwai <tiwai@suse.de>; Jaroslav Kysela <perex@perex.cz>; Mika > > Westerberg <mika.westerberg@linux.intel.com>; Bjorn Helgaas > > <helgaas@kernel.org>; Nicholas Johnson <nicholas.johnson- > > opensource@outlook.com.au>; alsa-devel@alsa-project.org; linux- > > kernel@vger.kernel.org; linux-pci@vger.kernel.org > > Subject: Re: [PATCH] ALSA: hda/hdmi - Fix duplicate unref of pci_dev > > > > On Tue, Dec 10, 2019 at 03:34:27PM +0000, Deucher, Alexander wrote: > > > > Nicholas Johnson reports a null pointer deref as well as a refcount > > > > underflow upon hot-removal of a Thunderbolt-attached AMD eGPU. > > > > He's bisected the issue down to commit 586bc4aab878 ("ALSA: hda/hdmi > > > > - fix vgaswitcheroo detection for AMD"). > > > > > > > > The commit iterates over PCI devices using pci_get_class() and > > > > unreferences each device found, even though pci_get_class() > > > > subsequently unreferences the device as well. Fix it. > > > > > > The pci_dev_put() a few lines above should probably be dropped as well. > > > > That one looks fine to me. The refcount is already increased in the caller > > get_bound_vga() via pci_get_domain_bus_and_slot() and it's increased > > again in atpx_present() via pci_get_class(). It needs to be decremented in > > atpx_present() to avoid leaking a ref. > > I'm not following. This is part of the same loop as the one you removed. All we are doing is checking whether the ATPX method exists or not om the platform. The pdev may not be the same one as the one in pci_get_domain_bus_and_slot(). The APTX method in the APU's ACPI namespace, not the dGPUs. Well, the tricky part is that pci_get_class() itself does unrefeference the old object and reference the new object (if found). At the end of the loop, nothing is referenced, so it's fine. OTOH, if you go out of the loop in the middle, you're still keeping the pdev object reference, so you need to manually unreference it. Takashi > > Alex > > > > > Thanks, > > > > Lukas > > > > > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c > > > > index 35b4526f0d28..b856b89378ac 100644 > > > > --- a/sound/pci/hda/hda_intel.c > > > > +++ b/sound/pci/hda/hda_intel.c > > > > @@ -1419,7 +1419,6 @@ static bool atpx_present(void) > > > > return true; > > > > } > > > > } > > > > - pci_dev_put(pdev); > > > > } > > > > return false; > > > > } > > > > -- > > > > 2.24.0 >
WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de> To: "Deucher, Alexander" <Alexander.Deucher@amd.com> Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>, Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>, "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Lukas Wunner <lukas@wunner.de>, Bjorn Helgaas <helgaas@kernel.org>, Mika Westerberg <mika.westerberg@linux.intel.com> Subject: Re: [alsa-devel] [PATCH] ALSA: hda/hdmi - Fix duplicate unref of pci_dev Date: Tue, 10 Dec 2019 17:10:40 +0100 [thread overview] Message-ID: <s5h4ky8w48v.wl-tiwai@suse.de> (raw) In-Reply-To: <MWHPR12MB1358449C677259C848AAB11EF75B0@MWHPR12MB1358.namprd12.prod.outlook.com> On Tue, 10 Dec 2019 16:53:20 +0100, Deucher, Alexander wrote: > > > -----Original Message----- > > From: Lukas Wunner <lukas@wunner.de> > > Sent: Tuesday, December 10, 2019 10:47 AM > > To: Deucher, Alexander <Alexander.Deucher@amd.com> > > Cc: Takashi Iwai <tiwai@suse.de>; Jaroslav Kysela <perex@perex.cz>; Mika > > Westerberg <mika.westerberg@linux.intel.com>; Bjorn Helgaas > > <helgaas@kernel.org>; Nicholas Johnson <nicholas.johnson- > > opensource@outlook.com.au>; alsa-devel@alsa-project.org; linux- > > kernel@vger.kernel.org; linux-pci@vger.kernel.org > > Subject: Re: [PATCH] ALSA: hda/hdmi - Fix duplicate unref of pci_dev > > > > On Tue, Dec 10, 2019 at 03:34:27PM +0000, Deucher, Alexander wrote: > > > > Nicholas Johnson reports a null pointer deref as well as a refcount > > > > underflow upon hot-removal of a Thunderbolt-attached AMD eGPU. > > > > He's bisected the issue down to commit 586bc4aab878 ("ALSA: hda/hdmi > > > > - fix vgaswitcheroo detection for AMD"). > > > > > > > > The commit iterates over PCI devices using pci_get_class() and > > > > unreferences each device found, even though pci_get_class() > > > > subsequently unreferences the device as well. Fix it. > > > > > > The pci_dev_put() a few lines above should probably be dropped as well. > > > > That one looks fine to me. The refcount is already increased in the caller > > get_bound_vga() via pci_get_domain_bus_and_slot() and it's increased > > again in atpx_present() via pci_get_class(). It needs to be decremented in > > atpx_present() to avoid leaking a ref. > > I'm not following. This is part of the same loop as the one you removed. All we are doing is checking whether the ATPX method exists or not om the platform. The pdev may not be the same one as the one in pci_get_domain_bus_and_slot(). The APTX method in the APU's ACPI namespace, not the dGPUs. Well, the tricky part is that pci_get_class() itself does unrefeference the old object and reference the new object (if found). At the end of the loop, nothing is referenced, so it's fine. OTOH, if you go out of the loop in the middle, you're still keeping the pdev object reference, so you need to manually unreference it. Takashi > > Alex > > > > > Thanks, > > > > Lukas > > > > > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c > > > > index 35b4526f0d28..b856b89378ac 100644 > > > > --- a/sound/pci/hda/hda_intel.c > > > > +++ b/sound/pci/hda/hda_intel.c > > > > @@ -1419,7 +1419,6 @@ static bool atpx_present(void) > > > > return true; > > > > } > > > > } > > > > - pci_dev_put(pdev); > > > > } > > > > return false; > > > > } > > > > -- > > > > 2.24.0 > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
next prev parent reply other threads:[~2019-12-10 16:10 UTC|newest] Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-09 12:34 Linux v5.5 serious PCI bug Nicholas Johnson 2019-12-09 12:37 ` Pavel Machek 2019-12-09 13:07 ` Nicholas Johnson 2019-12-09 13:12 ` mika.westerberg 2019-12-09 13:29 ` Nicholas Johnson 2019-12-09 13:33 ` Nicholas Johnson 2019-12-10 7:28 ` mika.westerberg 2019-12-10 12:00 ` Nicholas Johnson 2019-12-10 12:29 ` Lukas Wunner 2019-12-10 12:46 ` Takashi Iwai 2019-12-11 7:33 ` Jiasen Lin 2019-12-10 12:52 ` Nicholas Johnson 2019-12-10 12:34 ` mika.westerberg 2019-12-10 13:39 ` [PATCH] ALSA: hda/hdmi - Fix duplicate unref of pci_dev Lukas Wunner 2019-12-10 13:39 ` [alsa-devel] " Lukas Wunner 2019-12-10 13:41 ` Takashi Iwai 2019-12-10 13:41 ` [alsa-devel] " Takashi Iwai 2019-12-10 13:47 ` Nicholas Johnson 2019-12-10 13:47 ` [alsa-devel] " Nicholas Johnson 2019-12-10 13:50 ` Takashi Iwai 2019-12-10 13:50 ` [alsa-devel] " Takashi Iwai 2019-12-10 15:34 ` Deucher, Alexander 2019-12-10 15:34 ` [alsa-devel] " Deucher, Alexander 2019-12-10 15:46 ` Lukas Wunner 2019-12-10 15:46 ` [alsa-devel] " Lukas Wunner 2019-12-10 15:53 ` Deucher, Alexander 2019-12-10 15:53 ` [alsa-devel] " Deucher, Alexander 2019-12-10 16:10 ` Takashi Iwai [this message] 2019-12-10 16:10 ` Takashi Iwai 2019-12-10 16:51 ` Deucher, Alexander 2019-12-10 16:51 ` [alsa-devel] " Deucher, Alexander 2019-12-10 16:13 ` Lukas Wunner 2019-12-10 16:13 ` [alsa-devel] " Lukas Wunner
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=s5h4ky8w48v.wl-tiwai@suse.de \ --to=tiwai@suse.de \ --cc=Alexander.Deucher@amd.com \ --cc=alsa-devel@alsa-project.org \ --cc=helgaas@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=lukas@wunner.de \ --cc=mika.westerberg@linux.intel.com \ --cc=nicholas.johnson-opensource@outlook.com.au \ --cc=perex@perex.cz \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.