[U-Boot] [PATCH] timer: Fix possible underflow in udelay
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)
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)
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
participants (2)
-
Andrew F. Davis -
Troy Kisky