diff mbox series

[5/9] ALSA: hda: Skip event processing for unregistered codecs

Message ID 20220706120230.427296-6-cezary.rojewski@intel.com (mailing list archive)
State New, archived
Headers show
Series ALSA: hda: Codec-reload bug fixes and cleanups | expand

Commit Message

Cezary Rojewski July 6, 2022, 12:02 p.m. UTC
When codec is unbound but not yet removed, in the eyes of
snd_hdac_bus_process_unsol_events() it is still a valid target to
delegate work to. Such behaviour may lead to use-after-free errors.
Address by verifying if codec is actually registered.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 include/sound/hda_codec.h |  1 -
 include/sound/hdaudio.h   |  1 +
 sound/hda/hdac_bus.c      |  2 +-
 sound/pci/hda/hda_codec.c | 10 +++++-----
 sound/soc/codecs/hda.c    |  4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)

Comments

Takashi Iwai July 9, 2022, 4:47 p.m. UTC | #1
On Wed, 06 Jul 2022 14:02:26 +0200,
Cezary Rojewski wrote:
> 
> When codec is unbound but not yet removed, in the eyes of
> snd_hdac_bus_process_unsol_events() it is still a valid target to
> delegate work to. Such behaviour may lead to use-after-free errors.
> Address by verifying if codec is actually registered.
> 
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> ---
>  include/sound/hda_codec.h |  1 -
>  include/sound/hdaudio.h   |  1 +
>  sound/hda/hdac_bus.c      |  2 +-
>  sound/pci/hda/hda_codec.c | 10 +++++-----
>  sound/soc/codecs/hda.c    |  4 ++--

This patch doesn't apply cleanly because the change in
sound/soc/codecs/hda.c still not included in my tree.

Mark, there's yet one case we need the merge of asoc-next :)


thanks,

Takashi
Takashi Iwai July 15, 2022, 2:27 p.m. UTC | #2
On Sat, 09 Jul 2022 18:47:41 +0200,
Takashi Iwai wrote:
> 
> On Wed, 06 Jul 2022 14:02:26 +0200,
> Cezary Rojewski wrote:
> > 
> > When codec is unbound but not yet removed, in the eyes of
> > snd_hdac_bus_process_unsol_events() it is still a valid target to
> > delegate work to. Such behaviour may lead to use-after-free errors.
> > Address by verifying if codec is actually registered.
> > 
> > Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> > ---
> >  include/sound/hda_codec.h |  1 -
> >  include/sound/hdaudio.h   |  1 +
> >  sound/hda/hdac_bus.c      |  2 +-
> >  sound/pci/hda/hda_codec.c | 10 +++++-----
> >  sound/soc/codecs/hda.c    |  4 ++--
> 
> This patch doesn't apply cleanly because the change in
> sound/soc/codecs/hda.c still not included in my tree.

Now merged to for-next branch.


thanks,

Takashi
diff mbox series

Patch

diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index b7be300b6b18..6d3c82c4b6ac 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -231,7 +231,6 @@  struct hda_codec {
 	/* misc flags */
 	unsigned int configured:1; /* codec was configured */
 	unsigned int in_freeing:1; /* being released */
-	unsigned int registered:1; /* codec was registered */
 	unsigned int display_power_control:1; /* needs display power */
 	unsigned int spdif_status_reset :1; /* needs to toggle SPDIF for each
 					     * status change
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 15f15075238d..797bf67a164d 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -93,6 +93,7 @@  struct hdac_device {
 	bool lazy_cache:1;	/* don't wake up for writes */
 	bool caps_overwriting:1; /* caps overwrite being in process */
 	bool cache_coef:1;	/* cache COEF read/write too */
+	unsigned int registered:1; /* codec was registered */
 };
 
 /* device/driver type used for matching */
diff --git a/sound/hda/hdac_bus.c b/sound/hda/hdac_bus.c
index 71db8592b33d..d497414a5538 100644
--- a/sound/hda/hdac_bus.c
+++ b/sound/hda/hdac_bus.c
@@ -183,7 +183,7 @@  static void snd_hdac_bus_process_unsol_events(struct work_struct *work)
 		if (!(caddr & (1 << 4))) /* no unsolicited event? */
 			continue;
 		codec = bus->caddr_tbl[caddr & 0x0f];
-		if (!codec || !codec->dev.driver)
+		if (!codec || !codec->registered)
 			continue;
 		spin_unlock_irq(&bus->reg_lock);
 		drv = drv_to_hdac_driver(codec->dev.driver);
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 2381aced492f..75e85bf58681 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -772,11 +772,11 @@  static void codec_release_pcms(struct hda_codec *codec)
  */
 void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec)
 {
-	if (codec->registered) {
+	if (codec->core.registered) {
 		/* pm_runtime_put() is called in snd_hdac_device_exit() */
 		pm_runtime_get_noresume(hda_codec_dev(codec));
 		pm_runtime_disable(hda_codec_dev(codec));
-		codec->registered = 0;
+		codec->core.registered = 0;
 	}
 
 	snd_hda_codec_disconnect_pcms(codec);
@@ -824,14 +824,14 @@  void snd_hda_codec_display_power(struct hda_codec *codec, bool enable)
  */
 void snd_hda_codec_register(struct hda_codec *codec)
 {
-	if (codec->registered)
+	if (codec->core.registered)
 		return;
 	if (device_is_registered(hda_codec_dev(codec))) {
 		snd_hda_codec_display_power(codec, true);
 		pm_runtime_enable(hda_codec_dev(codec));
 		/* it was powered up in snd_hda_codec_new(), now all done */
 		snd_hda_power_down(codec);
-		codec->registered = 1;
+		codec->core.registered = 1;
 	}
 }
 EXPORT_SYMBOL_GPL(snd_hda_codec_register);
@@ -3047,7 +3047,7 @@  void snd_hda_codec_shutdown(struct hda_codec *codec)
 	struct hda_pcm *cpcm;
 
 	/* Skip the shutdown if codec is not registered */
-	if (!codec->registered)
+	if (!codec->core.registered)
 		return;
 
 	cancel_delayed_work_sync(&codec->jackpoll_work);
diff --git a/sound/soc/codecs/hda.c b/sound/soc/codecs/hda.c
index edcb8bc6806b..ad20a3dff9b7 100644
--- a/sound/soc/codecs/hda.c
+++ b/sound/soc/codecs/hda.c
@@ -274,7 +274,7 @@  static void hda_codec_remove(struct snd_soc_component *component)
 	struct hdac_device *hdev = &codec->core;
 	struct hdac_bus *bus = hdev->bus;
 	struct hdac_ext_link *hlink;
-	bool was_registered = codec->registered;
+	bool was_registered = codec->core.registered;
 
 	/* Don't allow any more runtime suspends */
 	pm_runtime_forbid(&hdev->dev);
@@ -376,7 +376,7 @@  static int hda_hdev_detach(struct hdac_device *hdev)
 {
 	struct hda_codec *codec = dev_to_hda_codec(&hdev->dev);
 
-	if (codec->registered)
+	if (codec->core.registered)
 		cancel_delayed_work_sync(&codec->jackpoll_work);
 
 	snd_soc_unregister_component(&hdev->dev);