All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -next v2] ALSA: timer: Fix use-after-free problem
@ 2021-11-03  3:35 ` Wang Wensheng
  0 siblings, 0 replies; 6+ messages in thread
From: Wang Wensheng @ 2021-11-03  3:35 UTC (permalink / raw)
  To: perex, tiwai, wangwensheng4, broonie, joe, alsa-devel, linux-kernel
  Cc: rui.xiang

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 delete the timer instance from ack_list and
active_list unconditionally in snd_timer_stop1().

Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
Suggested-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/timer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/core/timer.c b/sound/core/timer.c
index 92b7008fcdb8..4f9bab931951 100644
--- 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) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH -next v2] ALSA: timer: Fix use-after-free problem
@ 2021-11-03  3:35 ` Wang Wensheng
  0 siblings, 0 replies; 6+ messages in thread
From: Wang Wensheng @ 2021-11-03  3:35 UTC (permalink / raw)
  To: perex, tiwai, wangwensheng4, broonie, joe, alsa-devel, linux-kernel
  Cc: rui.xiang

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 delete the timer instance from ack_list and
active_list unconditionally in snd_timer_stop1().

Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
Suggested-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/timer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/core/timer.c b/sound/core/timer.c
index 92b7008fcdb8..4f9bab931951 100644
--- 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) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH -next v2] ALSA: timer: Fix use-after-free problem
  2021-11-03  3:35 ` Wang Wensheng
@ 2021-11-03 15:52   ` Takashi Iwai
  -1 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-11-03 15:52 UTC (permalink / raw)
  To: Wang Wensheng
  Cc: perex, tiwai, broonie, joe, alsa-devel, linux-kernel, rui.xiang

On Wed, 03 Nov 2021 04:35:17 +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 delete the timer instance from ack_list and
> active_list unconditionally in snd_timer_stop1().
> 
> Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
> Suggested-by: Takashi Iwai <tiwai@suse.de>

Thanks, applied now.


Takashi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH -next v2] ALSA: timer: Fix use-after-free problem
@ 2021-11-03 15:52   ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-11-03 15:52 UTC (permalink / raw)
  To: Wang Wensheng; +Cc: alsa-devel, linux-kernel, tiwai, rui.xiang, broonie, joe

On Wed, 03 Nov 2021 04:35:17 +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 delete the timer instance from ack_list and
> active_list unconditionally in snd_timer_stop1().
> 
> Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
> Suggested-by: Takashi Iwai <tiwai@suse.de>

Thanks, applied now.


Takashi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH -next v2] ALSA: timer: Fix use-after-free problem
  2021-11-03 15:52   ` Takashi Iwai
@ 2021-11-03 16:06     ` Takashi Iwai
  -1 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-11-03 16:06 UTC (permalink / raw)
  To: Wang Wensheng
  Cc: perex, tiwai, broonie, joe, alsa-devel, linux-kernel, rui.xiang

On Wed, 03 Nov 2021 16:52:31 +0100,
Takashi Iwai wrote:
> 
> On Wed, 03 Nov 2021 04:35:17 +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 delete the timer instance from ack_list and
> > active_list unconditionally in snd_timer_stop1().
> > 
> > Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
> > Suggested-by: Takashi Iwai <tiwai@suse.de>
> 
> Thanks, applied now.

BTW, while reviewing the patch, I noticed that we have also the
similar code path for a slave timer instance that has the same kind of
linked list entries.  I'll submit the corresponding fix patch.


Takashi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH -next v2] ALSA: timer: Fix use-after-free problem
@ 2021-11-03 16:06     ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-11-03 16:06 UTC (permalink / raw)
  To: Wang Wensheng; +Cc: alsa-devel, linux-kernel, tiwai, rui.xiang, broonie, joe

On Wed, 03 Nov 2021 16:52:31 +0100,
Takashi Iwai wrote:
> 
> On Wed, 03 Nov 2021 04:35:17 +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 delete the timer instance from ack_list and
> > active_list unconditionally in snd_timer_stop1().
> > 
> > Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
> > Suggested-by: Takashi Iwai <tiwai@suse.de>
> 
> Thanks, applied now.

BTW, while reviewing the patch, I noticed that we have also the
similar code path for a slave timer instance that has the same kind of
linked list entries.  I'll submit the corresponding fix patch.


Takashi

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-03 16:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-03  3:35 [PATCH -next v2] ALSA: timer: Fix use-after-free problem Wang Wensheng
2021-11-03  3:35 ` Wang Wensheng
2021-11-03 15:52 ` Takashi Iwai
2021-11-03 15:52   ` Takashi Iwai
2021-11-03 16:06   ` Takashi Iwai
2021-11-03 16:06     ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.