timer: Fix possible underflow in udelay

Message ID 20160825184058.2071-1-afd@ti.com
State New
Headers show

Commit Message

Andrew F. Davis Aug. 25, 2016, 6:40 p.m.
When the inputed usec is too large we process it in chunks of
CONFIG_WD_PERIOD size. Subtracting this from usec until usec is
zero. If usec is not an integer multiple of CONFIG_WD_PERIOD it
will underflow and the condition will not become false when it
should. Fix this logic.

Signed-off-by: Andrew F. Davis <afd@ti.com>

---
 lib/time.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
2.9.3

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Andrew F. Davis Aug. 25, 2016, 7:18 p.m. | #1
On 08/25/2016 02:02 PM, Troy Kisky wrote:
> On 8/25/2016 11:40 AM, Andrew F. Davis wrote:

>> When the inputed usec is too large we process it in chunks of

>> CONFIG_WD_PERIOD size. Subtracting this from usec until usec is

>> zero. If usec is not an integer multiple of CONFIG_WD_PERIOD it

>> will underflow and the condition will not become false when it

>> should. Fix this logic.

>>

>> Signed-off-by: Andrew F. Davis <afd@ti.com>

>> ---

>>  lib/time.c | 10 +++++-----

>>  1 file changed, 5 insertions(+), 5 deletions(-)

>>

>> diff --git a/lib/time.c b/lib/time.c

>> index f37150f..4ec3cb9 100644

>> --- a/lib/time.c

>> +++ b/lib/time.c

>> @@ -145,14 +145,14 @@ void __weak __udelay(unsigned long usec)

>>  

>>  void udelay(unsigned long usec)

>>  {

>> -	ulong kv;

>> +	ulong kv = usec > CONFIG_WD_PERIOD ? CONFIG_WD_PERIOD : usec;

>> +	ulong elapsed = 0;

>>  

>>  	do {

>>  		WATCHDOG_RESET();

>> -		kv = usec > CONFIG_WD_PERIOD ? CONFIG_WD_PERIOD : usec;

>> -		__udelay (kv);

>> -		usec -= kv;

>> -	} while(usec);

>> +		__udelay(kv);

>> +		elapsed += kv;

>> +	} while (elapsed < usec);

>>  }

>>  

>>  void mdelay(unsigned long msec)

>>

> 

> The original code looks fine to me. Can you give an example of failure ?

> ie.

> If udelay is passed value of CONFIG_WD_PERIOD+1, the udelay sequence will be

> 

> udelay(CONFIG_WD_PERIOD)

> udelay(1)

> 

> whereas the need code does

> udelay(CONFIG_WD_PERIOD)

> udelay(CONFIG_WD_PERIOD)

> 


Hmm, I'm not sure where I saw the problem before, I think I may have
tried to optimize it and broke it myself, oh well, this patch can safely
be ignored.

Thanks,
Andrew
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Patch

diff --git a/lib/time.c b/lib/time.c
index f37150f..4ec3cb9 100644
--- a/lib/time.c
+++ b/lib/time.c
@@ -145,14 +145,14 @@  void __weak __udelay(unsigned long usec)
 
 void udelay(unsigned long usec)
 {
-	ulong kv;
+	ulong kv = usec > CONFIG_WD_PERIOD ? CONFIG_WD_PERIOD : usec;
+	ulong elapsed = 0;
 
 	do {
 		WATCHDOG_RESET();
-		kv = usec > CONFIG_WD_PERIOD ? CONFIG_WD_PERIOD : usec;
-		__udelay (kv);
-		usec -= kv;
-	} while(usec);
+		__udelay(kv);
+		elapsed += kv;
+	} while (elapsed < usec);
 }
 
 void mdelay(unsigned long msec)