UBUNTU (pre-stable) Natty: ALSA: pcm: fix infinite loop in snd_pcm_update_hw_ptr0()

Submitted by David Henningsson on April 5, 2011, 5:46 a.m.

Details

Message ID 4D9AACC2.1080708@canonical.com
State New
Headers show

Commit Message

David Henningsson April 5, 2011, 5:46 a.m.
While this hasn't reached all trees yet, I think we should pick it up 
ASAP, due to what it prevents: hard lockups with interrupts disabled.

Comments

Tim Gardner April 5, 2011, 12:46 p.m.
On 04/04/2011 11:46 PM, David Henningsson wrote:
> While this hasn't reached all trees yet, I think we should pick it up
> ASAP, due to what it prevents: hard lockups with interrupts disabled.
>

Applied to Natty. Do you think its necessary for older kernels?
David Henningsson April 5, 2011, 12:47 p.m.
On 2011-04-05 14:46, Tim Gardner wrote:
> On 04/04/2011 11:46 PM, David Henningsson wrote:
>> While this hasn't reached all trees yet, I think we should pick it up
>> ASAP, due to what it prevents: hard lockups with interrupts disabled.
>>
>
> Applied to Natty. Do you think its necessary for older kernels?
>

As I understand it, this was a 2.6.38 regression.

Patch hide | download patch | download mbox

From 12ff414e2e4512f59fe191dc18e856e2939a1c79 Mon Sep 17 00:00:00 2001
From: Kelly Anderson <kelly@silka.with-linux.com>
Date: Fri, 1 Apr 2011 11:58:25 +0200
Subject: [PATCH] ALSA: pcm: fix infinite loop in snd_pcm_update_hw_ptr0()

When period interrupts are disabled, snd_pcm_update_hw_ptr0() compares
the current time against the time estimated for the current hardware
pointer to detect xruns.  The somewhat fuzzy threshold in the while loop
makes it possible that hdelta becomes negative; the comparison being
done with unsigned types then makes the loop go through the entire 263
negative range, and, depending on the value, never reach an unsigned
value that is small enough to stop the loop.  Doing this with interrupts
disabled results in the machine locking up.

To prevent this, ensure that the loop condition uses signed types for
both operands so that the comparison is correctly done.

Many thanks to Kelly Anderson for debugging this.

Reported-by: Nix <nix@esperi.org.uk>
Reported-by: "Christopher K." <c.krooss@googlemail.com>
Reported-and-tested-by: Kelly Anderson <kelly@silka.with-linux.com>
Signed-off-by: Kelly Anderson <kelly@silka.with-linux.com>
[cl: remove unneeded casts; use a temp variable]
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Cc: 2.6.38 <stable@kernel.org>

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/pcm_lib.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index a82e3756..64449cb 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -375,6 +375,7 @@  static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 	}
 
 	if (runtime->no_period_wakeup) {
+		snd_pcm_sframes_t xrun_threshold;
 		/*
 		 * Without regular period interrupts, we have to check
 		 * the elapsed time to detect xruns.
@@ -383,7 +384,8 @@  static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 		if (jdelta < runtime->hw_ptr_buffer_jiffies / 2)
 			goto no_delta_check;
 		hdelta = jdelta - delta * HZ / runtime->rate;
-		while (hdelta > runtime->hw_ptr_buffer_jiffies / 2 + 1) {
+		xrun_threshold = runtime->hw_ptr_buffer_jiffies / 2 + 1;
+		while (hdelta > xrun_threshold) {
 			delta += runtime->buffer_size;
 			hw_base += runtime->buffer_size;
 			if (hw_base >= runtime->boundary)
-- 
1.7.4.1