On 2022-04-01 2:29 PM, 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
Nitpick: "when using ASoC" is quite a generic statement. By that I guess you relate to concept of splitting audio functionality into smaller logical blocks - components (struct snd_soc_components) - and the possible synchronization issues that are part of that division.
That alone is probably not a reason for re-send so:
Reviewed-by: Cezary Rojewski cezary.rojewski@intel.com
include/sound/jack.h | 1 + sound/core/jack.c | 37 ++++++++++++++++++++++++++++++------- 2 files changed, 31 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..58b9ebf5e7e1 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,11 +525,15 @@ 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;
mutex_lock(&jack->input_dev_lock);
jack->input_dev = input_allocate_device(); if (jack->input_dev == NULL) { err = -ENOMEM;
@@ -537,8 +549,9 @@ 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 */
}mutex_unlock(&jack->input_dev_lock);
+#endif /* CONFIG_SND_JACK_INPUT_DEV */
err = snd_device_new(card, SNDRV_DEV_JACK, jack, &ops); if (err < 0) @@ -556,7 +569,9 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
fail_input: #ifdef CONFIG_SND_JACK_INPUT_DEV
- mutex_lock(&jack->input_dev_lock); input_free_device(jack->input_dev);
- mutex_unlock(&jack->input_dev_lock); #endif kfree(jack->id); kfree(jack);
@@ -578,10 +593,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);
@@ -654,8 +673,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 +697,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);