linux-kernel - Re: [PATCH] ali5451: fix timeout handling in snd_ali_{codecs,timer}

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200908231527.25987.bzolnier@gmail.com>
Date:	Sun, 23 Aug 2009 15:27:25 +0200
From:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To:	Takashi Iwai <tiwai@...e.de>
Cc:	Jack Byer <ojbyer@....net>, alsa-devel@...a-project.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready()

On Thursday 20 August 2009 08:33:21 Takashi Iwai wrote:
> At Wed, 19 Aug 2009 22:09:50 +0200,
> Bartlomiej Zolnierkiewicz wrote:
> > 
> > From: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
> > Subject: [PATCH] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready()
> > 
> > Modify loops in such way that the register value is checked also after
> > the timeout condition, just in case the heavy interrupt load etc. caused
> > the thread to sleep for the time period exceeding the timeout value.
> > 
> > While at it remove an extra ALI_STIMER read from snd_ali_stimer_ready().
> > 
> > Reported-by: Jack Byer <ojbyer@....net>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
> > ---
> >  sound/pci/ali5451/ali5451.c |   11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > Index: b/sound/pci/ali5451/ali5451.c
> > ===================================================================
> > --- a/sound/pci/ali5451/ali5451.c
> > +++ b/sound/pci/ali5451/ali5451.c
> > @@ -314,8 +314,11 @@ static int snd_ali_codec_ready(struct sn
> >  		res = snd_ali_5451_peek(codec,port);
> >  		if (!(res & 0x8000))
> >  			return 0;
> > +		if (!time_after_eq(end_time, jiffies))
> > +			break;
> >  		schedule_timeout_uninterruptible(1);
> > -	} while (time_after_eq(end_time, jiffies));
> > +	} while (1);
> 
> Using for (;;) is more generic.  I see your patch keeps the changes
> minimal, but I'm afraid that the result, do {} while(1), can be
> misleading.
> 
> Could you replace with for (;;) ?

Sure, though I don't see a much value added from this operation..

From: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
Subject: [PATCH v2] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready()

Modify loops in such way that the register value is checked also after
the timeout condition, just in case the heavy interrupt load etc. caused
the thread to sleep for the time period exceeding the timeout value.

While at it remove an extra ALI_STIMER read from snd_ali_stimer_ready().

Reported-by: Jack Byer <ojbyer@....net>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
---
 sound/pci/ali5451/ali5451.c |   18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Index: b/sound/pci/ali5451/ali5451.c
===================================================================
--- a/sound/pci/ali5451/ali5451.c
+++ b/sound/pci/ali5451/ali5451.c
@@ -310,12 +310,16 @@ static int snd_ali_codec_ready(struct sn
 	unsigned int res;
 	
 	end_time = jiffies + msecs_to_jiffies(250);
-	do {
+
+	for (;;) {
 		res = snd_ali_5451_peek(codec,port);
 		if (!(res & 0x8000))
 			return 0;
+		if (!time_after_eq(end_time, jiffies))
+			break;
 		schedule_timeout_uninterruptible(1);
-	} while (time_after_eq(end_time, jiffies));
+	}
+
 	snd_ali_5451_poke(codec, port, res & ~0x8000);
 	snd_printdd("ali_codec_ready: codec is not ready.\n ");
 	return -EIO;
@@ -327,15 +331,17 @@ static int snd_ali_stimer_ready(struct s
 	unsigned long dwChk1,dwChk2;
 	
 	dwChk1 = snd_ali_5451_peek(codec, ALI_STIMER);
-	dwChk2 = snd_ali_5451_peek(codec, ALI_STIMER);
-
 	end_time = jiffies + msecs_to_jiffies(250);
-	do {
+
+	for (;;) {
 		dwChk2 = snd_ali_5451_peek(codec, ALI_STIMER);
 		if (dwChk2 != dwChk1)
 			return 0;
+		if (!time_after_eq(end_time, jiffies))
+			break;
 		schedule_timeout_uninterruptible(1);
-	} while (time_after_eq(end_time, jiffies));
+	}
+
 	snd_printk(KERN_ERR "ali_stimer_read: stimer is not ready.\n");
 	return -EIO;
 }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/