[PATCH v2] ALSA: jack: Access input_dev under mutex
It is possible when using ASoC that input_dev is unregistered while calling snd_jack_report, which causes NULL pointer dereference. In order to prevent this serialize access to input_dev using mutex lock.
Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com ---
Changes: v2: - drop mutex in snd_jack_new - add comment that snd_jack_report() should be called in context where it can sleep
--- include/sound/jack.h | 1 + sound/core/jack.c | 34 +++++++++++++++++++++++++++------- 2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/include/sound/jack.h b/include/sound/jack.h index 1181f536557e..1ed90e2109e9 100644 --- a/include/sound/jack.h +++ b/include/sound/jack.h @@ -62,6 +62,7 @@ struct snd_jack { const char *id; #ifdef CONFIG_SND_JACK_INPUT_DEV struct input_dev *input_dev; + struct mutex input_dev_lock; int registered; int type; char name[100]; diff --git a/sound/core/jack.c b/sound/core/jack.c index d1e3055f2b6a..88493cc31914 100644 --- a/sound/core/jack.c +++ b/sound/core/jack.c @@ -42,8 +42,11 @@ static int snd_jack_dev_disconnect(struct snd_device *device) #ifdef CONFIG_SND_JACK_INPUT_DEV struct snd_jack *jack = device->device_data;
- if (!jack->input_dev) + mutex_lock(&jack->input_dev_lock); + if (!jack->input_dev) { + mutex_unlock(&jack->input_dev_lock); return 0; + }
/* If the input device is registered with the input subsystem * then we need to use a different deallocator. */ @@ -52,6 +55,7 @@ static int snd_jack_dev_disconnect(struct snd_device *device) else input_free_device(jack->input_dev); jack->input_dev = NULL; + mutex_unlock(&jack->input_dev_lock); #endif /* CONFIG_SND_JACK_INPUT_DEV */ return 0; } @@ -90,8 +94,11 @@ static int snd_jack_dev_register(struct snd_device *device) snprintf(jack->name, sizeof(jack->name), "%s %s", card->shortname, jack->id);
- if (!jack->input_dev) + mutex_lock(&jack->input_dev_lock); + if (!jack->input_dev) { + mutex_unlock(&jack->input_dev_lock); return 0; + }
jack->input_dev->name = jack->name;
@@ -116,6 +123,7 @@ static int snd_jack_dev_register(struct snd_device *device) if (err == 0) jack->registered = 1;
+ mutex_unlock(&jack->input_dev_lock); return err; } #endif /* CONFIG_SND_JACK_INPUT_DEV */ @@ -517,9 +525,11 @@ int snd_jack_new(struct snd_card *card, const char *id, int type, return -ENOMEM; }
- /* don't creat input device for phantom jack */ - if (!phantom_jack) { #ifdef CONFIG_SND_JACK_INPUT_DEV + mutex_init(&jack->input_dev_lock); + + /* don't create input device for phantom jack */ + if (!phantom_jack) { int i;
jack->input_dev = input_allocate_device(); @@ -537,8 +547,8 @@ int snd_jack_new(struct snd_card *card, const char *id, int type, input_set_capability(jack->input_dev, EV_SW, jack_switch_types[i]);
-#endif /* CONFIG_SND_JACK_INPUT_DEV */ } +#endif /* CONFIG_SND_JACK_INPUT_DEV */
err = snd_device_new(card, SNDRV_DEV_JACK, jack, &ops); if (err < 0) @@ -578,10 +588,14 @@ EXPORT_SYMBOL(snd_jack_new); void snd_jack_set_parent(struct snd_jack *jack, struct device *parent) { WARN_ON(jack->registered); - if (!jack->input_dev) + mutex_lock(&jack->input_dev_lock); + if (!jack->input_dev) { + mutex_unlock(&jack->input_dev_lock); return; + }
jack->input_dev->dev.parent = parent; + mutex_unlock(&jack->input_dev_lock); } EXPORT_SYMBOL(snd_jack_set_parent);
@@ -629,6 +643,8 @@ EXPORT_SYMBOL(snd_jack_set_key);
/** * snd_jack_report - Report the current status of a jack + * Note: This function uses mutexes and should be called from a + * context which can sleep (such as a workqueue). * * @jack: The jack to report status for * @status: The current status of the jack @@ -654,8 +670,11 @@ void snd_jack_report(struct snd_jack *jack, int status) status & jack_kctl->mask_bits);
#ifdef CONFIG_SND_JACK_INPUT_DEV - if (!jack->input_dev) + mutex_lock(&jack->input_dev_lock); + if (!jack->input_dev) { + mutex_unlock(&jack->input_dev_lock); return; + }
for (i = 0; i < ARRAY_SIZE(jack->key); i++) { int testbit = ((SND_JACK_BTN_0 >> i) & ~mask_bits); @@ -675,6 +694,7 @@ void snd_jack_report(struct snd_jack *jack, int status) }
input_sync(jack->input_dev); + mutex_unlock(&jack->input_dev_lock); #endif /* CONFIG_SND_JACK_INPUT_DEV */ } EXPORT_SYMBOL(snd_jack_report);
On Tue, 12 Apr 2022 11:16:28 +0200, Amadeusz Sławiński wrote:
It is possible when using ASoC that input_dev is unregistered while calling snd_jack_report, which causes NULL pointer dereference. In order to prevent this serialize access to input_dev using mutex lock.
Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com
Changes: v2:
- drop mutex in snd_jack_new
- add comment that snd_jack_report() should be called in context where it can sleep
Thanks, applied now.
Takashi
On 2022-04-12 11:44 AM, Takashi Iwai wrote:
On Tue, 12 Apr 2022 11:16:28 +0200, Amadeusz Sławiński wrote:
It is possible when using ASoC that input_dev is unregistered while calling snd_jack_report, which causes NULL pointer dereference. In order to prevent this serialize access to input_dev using mutex lock.
Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com
Changes: v2:
- drop mutex in snd_jack_new
- add comment that snd_jack_report() should be called in context where it can sleep
Thanks, applied now.
If you would be needing a reviewed-by-tag, you can safely apply my (it's missing in v2 sent by Amadeo).
For transparency:
Reviewed-by: Cezary Rojewski cezary.rojewski@intel.com
Regards, Czarek
participants (3)
-
Amadeusz Sławiński
-
Cezary Rojewski
-
Takashi Iwai