在 2021/11/2 22:10, Takashi Iwai 写道:
On Tue, 02 Nov 2021 14:41:07 +0100, Wang Wensheng wrote:
When the timer instance was add into ack_list but was not currently in process, the user could stop it via snd_timer_stop1() without delete it from the ack_list. Then the user could free the timer instance and when it was actually processed UAF occurred.
This issue could be reproduced via testcase snd_timer01 in ltp - running several instances of that testcase at the same time.
What I actually met was that the ack_list of the timer broken and the kernel went into deadloop with irqoff. That could be detected by hardlockup detector on board or when we run it on qemu, we could use gdb to dump the ack_list when the console has no response.
To fix this issue, we introduce a new flag SNDRV_TIMER_IFLG_ACKING to indicate the state where the timer instance is in ack_list but not currently processed and check against the new flag in snd_timer_stop1() and delete it from ack_list if the flag is set.
Signed-off-by: Wang Wensheng wangwensheng4@huawei.com
Thanks for the patch. Just through a quick glance, I wonder whether it'd be easier to do list_del_init(&timeri->ack_list) unconditionally before the check of timeri->flags in snd_timer1_stop(). Ditto for active_list. So something like:
Thanks for your suggestions. It looks much more efficient and easier and is also effective. I will take this as a second version.
--- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -624,13 +624,13 @@ static int snd_timer_stop1(struct snd_timer_instance *timeri, bool stop) if (!timer) return -EINVAL; spin_lock_irqsave(&timer->lock, flags);
- list_del_init(&timeri->ack_list);
- list_del_init(&timeri->active_list); if (!(timeri->flags & (SNDRV_TIMER_IFLG_RUNNING | SNDRV_TIMER_IFLG_START))) { result = -EBUSY; goto unlock; }
- list_del_init(&timeri->ack_list);
- list_del_init(&timeri->active_list); if (timer->card && timer->card->shutdown) goto unlock; if (stop) {
Takashi