tmakin/ack-tegra
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ALSA: timer: Don't take register_mutex with copy_from/to_user()

Browse Source 
[ Upstream commit 3424c8f53bc63c87712a7fc22dc13d0cc85fb0d6 ]

The infamous mmap_lock taken in copy_from/to_user() can be often
problematic when it's called inside another mutex, as they might lead
to deadlocks.

In the case of ALSA timer code, the bad pattern is with
guard(mutex)(&register_mutex) that covers copy_from/to_user() -- which
was mistakenly introduced at converting to guard(), and it had been
carefully worked around in the past.

This patch fixes those pieces simply by moving copy_from/to_user() out
of the register mutex lock again.

Fixes: 3923de04c8 ("ALSA: pcm: oss: Use guard() for setup")
Reported-by: syzbot+2b96f44164236dda0f3b@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/67dd86c8.050a0220.25ae54.0059.GAE@google.com
Link: https://patch.msgid.link/20250321172653.14310-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Takashi Iwai
2025-03-21 18:26:52 +01:00
committed by Greg Kroah-Hartman
parent 11242f4b9b
commit 15291b561d
1 changed files with 77 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sound/core/timer.c
+77 -70
View File
@@ -1515,91 +1515,97 @@ static void snd_timer_user_copy_id(struct snd_timer_id *id, struct snd_timer *ti
id->subdevice = timer->tmr_subdevice; id->subdevice = timer->tmr_subdevice;
} }
static int snd_timer_user_next_device(struct snd_timer_id __user *_tid) static void get_next_device(struct snd_timer_id *id)
{ {
struct snd_timer_id id;
struct snd_timer *timer; struct snd_timer *timer;
struct list_head *p; struct list_head *p;
if (copy_from_user(&id, _tid, sizeof(id))) if (id->dev_class < 0) { /* first item */
return -EFAULT;
guard(mutex)(&register_mutex);
if (id.dev_class < 0) { /* first item */
if (list_empty(&snd_timer_list)) if (list_empty(&snd_timer_list))
snd_timer_user_zero_id(&id); snd_timer_user_zero_id(id);
else { else {
timer = list_entry(snd_timer_list.next, timer = list_entry(snd_timer_list.next,
struct snd_timer, device_list); struct snd_timer, device_list);
snd_timer_user_copy_id(&id, timer); snd_timer_user_copy_id(id, timer);
} }
} else { } else {
switch (id.dev_class) { switch (id->dev_class) {
case SNDRV_TIMER_CLASS_GLOBAL: case SNDRV_TIMER_CLASS_GLOBAL:
id.device = id.device < 0 ? 0 : id.device + 1; id->device = id->device < 0 ? 0 : id->device + 1;
list_for_each(p, &snd_timer_list) { list_for_each(p, &snd_timer_list) {
timer = list_entry(p, struct snd_timer, device_list); timer = list_entry(p, struct snd_timer, device_list);
if (timer->tmr_class > SNDRV_TIMER_CLASS_GLOBAL) { if (timer->tmr_class > SNDRV_TIMER_CLASS_GLOBAL) {
snd_timer_user_copy_id(&id, timer); snd_timer_user_copy_id(id, timer);
break; break;
} }
if (timer->tmr_device >= id.device) { if (timer->tmr_device >= id->device) {
snd_timer_user_copy_id(&id, timer); snd_timer_user_copy_id(id, timer);
break; break;
} }
} }
if (p == &snd_timer_list) if (p == &snd_timer_list)
snd_timer_user_zero_id(&id); snd_timer_user_zero_id(id);
break; break;
case SNDRV_TIMER_CLASS_CARD: case SNDRV_TIMER_CLASS_CARD:
case SNDRV_TIMER_CLASS_PCM: case SNDRV_TIMER_CLASS_PCM:
if (id.card < 0) { if (id->card < 0) {
id.card = 0; id->card = 0;
} else { } else {
if (id.device < 0) { if (id->device < 0) {
id.device = 0; id->device = 0;
} else { } else {
if (id.subdevice < 0) if (id->subdevice < 0)
id.subdevice = 0; id->subdevice = 0;
else if (id.subdevice < INT_MAX) else if (id->subdevice < INT_MAX)
id.subdevice++; id->subdevice++;
} }
} }
list_for_each(p, &snd_timer_list) { list_for_each(p, &snd_timer_list) {
timer = list_entry(p, struct snd_timer, device_list); timer = list_entry(p, struct snd_timer, device_list);
if (timer->tmr_class > id.dev_class) { if (timer->tmr_class > id->dev_class) {
snd_timer_user_copy_id(&id, timer); snd_timer_user_copy_id(id, timer);
break; break;
} }
if (timer->tmr_class < id.dev_class) if (timer->tmr_class < id->dev_class)
continue; continue;
if (timer->card->number > id.card) { if (timer->card->number > id->card) {
snd_timer_user_copy_id(&id, timer); snd_timer_user_copy_id(id, timer);
break; break;
} }
if (timer->card->number < id.card) if (timer->card->number < id->card)
continue; continue;
if (timer->tmr_device > id.device) { if (timer->tmr_device > id->device) {
snd_timer_user_copy_id(&id, timer); snd_timer_user_copy_id(id, timer);
break; break;
} }
if (timer->tmr_device < id.device) if (timer->tmr_device < id->device)
continue; continue;
if (timer->tmr_subdevice > id.subdevice) { if (timer->tmr_subdevice > id->subdevice) {
snd_timer_user_copy_id(&id, timer); snd_timer_user_copy_id(id, timer);
break; break;
} }
if (timer->tmr_subdevice < id.subdevice) if (timer->tmr_subdevice < id->subdevice)
continue; continue;
snd_timer_user_copy_id(&id, timer); snd_timer_user_copy_id(id, timer);
break; break;
} }
if (p == &snd_timer_list) if (p == &snd_timer_list)
snd_timer_user_zero_id(&id); snd_timer_user_zero_id(id);
break; break;
default: default:
snd_timer_user_zero_id(&id); snd_timer_user_zero_id(id);
} }
} }
}
static int snd_timer_user_next_device(struct snd_timer_id __user *_tid)
{
struct snd_timer_id id;
if (copy_from_user(&id, _tid, sizeof(id)))
return -EFAULT;
scoped_guard(mutex, &register_mutex)
get_next_device(&id);
if (copy_to_user(_tid, &id, sizeof(*_tid))) if (copy_to_user(_tid, &id, sizeof(*_tid)))
return -EFAULT; return -EFAULT;
return 0; return 0;
@@ -1620,23 +1626,24 @@ static int snd_timer_user_ginfo(struct file *file,
tid = ginfo->tid; tid = ginfo->tid;
memset(ginfo, 0, sizeof(*ginfo)); memset(ginfo, 0, sizeof(*ginfo));
ginfo->tid = tid; ginfo->tid = tid;
guard(mutex)(&register_mutex); scoped_guard(mutex, &register_mutex) {
t = snd_timer_find(&tid); t = snd_timer_find(&tid);
if (!t) if (!t)
return -ENODEV; return -ENODEV;
ginfo->card = t->card ? t->card->number : -1; ginfo->card = t->card ? t->card->number : -1;
if (t->hw.flags & SNDRV_TIMER_HW_SLAVE) if (t->hw.flags & SNDRV_TIMER_HW_SLAVE)
ginfo->flags |= SNDRV_TIMER_FLG_SLAVE; ginfo->flags |= SNDRV_TIMER_FLG_SLAVE;
strscpy(ginfo->id, t->id, sizeof(ginfo->id)); strscpy(ginfo->id, t->id, sizeof(ginfo->id));
strscpy(ginfo->name, t->name, sizeof(ginfo->name)); strscpy(ginfo->name, t->name, sizeof(ginfo->name));
scoped_guard(spinlock_irq, &t->lock) scoped_guard(spinlock_irq, &t->lock)
ginfo->resolution = snd_timer_hw_resolution(t); ginfo->resolution = snd_timer_hw_resolution(t);
if (t->hw.resolution_min > 0) { if (t->hw.resolution_min > 0) {
ginfo->resolution_min = t->hw.resolution_min; ginfo->resolution_min = t->hw.resolution_min;
ginfo->resolution_max = t->hw.resolution_max; ginfo->resolution_max = t->hw.resolution_max;
} }
list_for_each(p, &t->open_list_head) { list_for_each(p, &t->open_list_head) {
ginfo->clients++; ginfo->clients++;
}
} }
if (copy_to_user(_ginfo, ginfo, sizeof(*ginfo))) if (copy_to_user(_ginfo, ginfo, sizeof(*ginfo)))
return -EFAULT; return -EFAULT;
@@ -1674,31 +1681,31 @@ static int snd_timer_user_gstatus(struct file *file,
struct snd_timer_gstatus gstatus; struct snd_timer_gstatus gstatus;
struct snd_timer_id tid; struct snd_timer_id tid;
struct snd_timer *t; struct snd_timer *t;
int err = 0;
if (copy_from_user(&gstatus, _gstatus, sizeof(gstatus))) if (copy_from_user(&gstatus, _gstatus, sizeof(gstatus)))
return -EFAULT; return -EFAULT;
tid = gstatus.tid; tid = gstatus.tid;
memset(&gstatus, 0, sizeof(gstatus)); memset(&gstatus, 0, sizeof(gstatus));
gstatus.tid = tid; gstatus.tid = tid;
guard(mutex)(&register_mutex); scoped_guard(mutex, &register_mutex) {
t = snd_timer_find(&tid); t = snd_timer_find(&tid);
if (t != NULL) { if (t != NULL) {
guard(spinlock_irq)(&t->lock); guard(spinlock_irq)(&t->lock);
gstatus.resolution = snd_timer_hw_resolution(t); gstatus.resolution = snd_timer_hw_resolution(t);
if (t->hw.precise_resolution) { if (t->hw.precise_resolution) {
t->hw.precise_resolution(t, &gstatus.resolution_num, t->hw.precise_resolution(t, &gstatus.resolution_num,
&gstatus.resolution_den); &gstatus.resolution_den);
} else {
gstatus.resolution_num = gstatus.resolution;
gstatus.resolution_den = 1000000000uL;
}
} else { } else {
gstatus.resolution_num = gstatus.resolution; return -ENODEV;
gstatus.resolution_den = 1000000000uL;
} }
} else {
err = -ENODEV;
} }
if (err >= 0 && copy_to_user(_gstatus, &gstatus, sizeof(gstatus))) if (copy_to_user(_gstatus, &gstatus, sizeof(gstatus)))
err = -EFAULT; return -EFAULT;
return err; return 0;
} }
static int snd_timer_user_tselect(struct file *file, static int snd_timer_user_tselect(struct file *file,