Commit 6564d0ad authored by Takashi Iwai's avatar Takashi Iwai

ALSA: ctl: Workaround for lockdep warning wrt card->ctl_files_rwlock

The recent change in lockdep for read lock caused the deadlock
warnings in ALSA control code which uses the read_lock() for
notification and else while write_lock_irqsave() is used for adding
and removing the list entry.  Although a deadlock would practically
never hit in a real usage (the addition and the deletion can't happen
with the notification), it's better to fix the read_lock() usage in a
semantically correct way.

This patch replaces the read_lock() calls with read_lock_irqsave()
version for avoiding a reported deadlock.  The notification code path
takes the irq disablement in anyway, and other code paths are very
short execution, hence there shouldn't be any big performance hit by
this change.

Fixes: e9181886 ("locking: More accurate annotations for read_lock()")
Reported-by: syzbot+561a74f84100162990b2@syzkaller.appspotmail.com
Link: https://lore.kernel.org/r/20200922084953.29018-1-tiwai@suse.deSigned-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent 534ad9af
...@@ -150,14 +150,14 @@ void snd_ctl_notify(struct snd_card *card, unsigned int mask, ...@@ -150,14 +150,14 @@ void snd_ctl_notify(struct snd_card *card, unsigned int mask,
return; return;
if (card->shutdown) if (card->shutdown)
return; return;
read_lock(&card->ctl_files_rwlock); read_lock_irqsave(&card->ctl_files_rwlock, flags);
#if IS_ENABLED(CONFIG_SND_MIXER_OSS) #if IS_ENABLED(CONFIG_SND_MIXER_OSS)
card->mixer_oss_change_count++; card->mixer_oss_change_count++;
#endif #endif
list_for_each_entry(ctl, &card->ctl_files, list) { list_for_each_entry(ctl, &card->ctl_files, list) {
if (!ctl->subscribed) if (!ctl->subscribed)
continue; continue;
spin_lock_irqsave(&ctl->read_lock, flags); spin_lock(&ctl->read_lock);
list_for_each_entry(ev, &ctl->events, list) { list_for_each_entry(ev, &ctl->events, list) {
if (ev->id.numid == id->numid) { if (ev->id.numid == id->numid) {
ev->mask |= mask; ev->mask |= mask;
...@@ -174,10 +174,10 @@ void snd_ctl_notify(struct snd_card *card, unsigned int mask, ...@@ -174,10 +174,10 @@ void snd_ctl_notify(struct snd_card *card, unsigned int mask,
} }
_found: _found:
wake_up(&ctl->change_sleep); wake_up(&ctl->change_sleep);
spin_unlock_irqrestore(&ctl->read_lock, flags); spin_unlock(&ctl->read_lock);
kill_fasync(&ctl->fasync, SIGIO, POLL_IN); kill_fasync(&ctl->fasync, SIGIO, POLL_IN);
} }
read_unlock(&card->ctl_files_rwlock); read_unlock_irqrestore(&card->ctl_files_rwlock, flags);
} }
EXPORT_SYMBOL(snd_ctl_notify); EXPORT_SYMBOL(snd_ctl_notify);
...@@ -1951,8 +1951,9 @@ int snd_ctl_get_preferred_subdevice(struct snd_card *card, int type) ...@@ -1951,8 +1951,9 @@ int snd_ctl_get_preferred_subdevice(struct snd_card *card, int type)
{ {
struct snd_ctl_file *kctl; struct snd_ctl_file *kctl;
int subdevice = -1; int subdevice = -1;
unsigned long flags;
read_lock(&card->ctl_files_rwlock); read_lock_irqsave(&card->ctl_files_rwlock, flags);
list_for_each_entry(kctl, &card->ctl_files, list) { list_for_each_entry(kctl, &card->ctl_files, list) {
if (kctl->pid == task_pid(current)) { if (kctl->pid == task_pid(current)) {
subdevice = kctl->preferred_subdevice[type]; subdevice = kctl->preferred_subdevice[type];
...@@ -1960,7 +1961,7 @@ int snd_ctl_get_preferred_subdevice(struct snd_card *card, int type) ...@@ -1960,7 +1961,7 @@ int snd_ctl_get_preferred_subdevice(struct snd_card *card, int type)
break; break;
} }
} }
read_unlock(&card->ctl_files_rwlock); read_unlock_irqrestore(&card->ctl_files_rwlock, flags);
return subdevice; return subdevice;
} }
EXPORT_SYMBOL_GPL(snd_ctl_get_preferred_subdevice); EXPORT_SYMBOL_GPL(snd_ctl_get_preferred_subdevice);
...@@ -2009,13 +2010,14 @@ static int snd_ctl_dev_disconnect(struct snd_device *device) ...@@ -2009,13 +2010,14 @@ static int snd_ctl_dev_disconnect(struct snd_device *device)
{ {
struct snd_card *card = device->device_data; struct snd_card *card = device->device_data;
struct snd_ctl_file *ctl; struct snd_ctl_file *ctl;
unsigned long flags;
read_lock(&card->ctl_files_rwlock); read_lock_irqsave(&card->ctl_files_rwlock, flags);
list_for_each_entry(ctl, &card->ctl_files, list) { list_for_each_entry(ctl, &card->ctl_files, list) {
wake_up(&ctl->change_sleep); wake_up(&ctl->change_sleep);
kill_fasync(&ctl->fasync, SIGIO, POLL_ERR); kill_fasync(&ctl->fasync, SIGIO, POLL_ERR);
} }
read_unlock(&card->ctl_files_rwlock); read_unlock_irqrestore(&card->ctl_files_rwlock, flags);
return snd_unregister_device(&card->ctl_dev); return snd_unregister_device(&card->ctl_dev);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment