diff mbox series

[ovs-dev,v3,03/14] stopwatch: Fix buffer underflow when computing percentiles.

Message ID 20220124141750.11777.67888.stgit@dceara.remote.csb
State Accepted
Commit b07c2e92e9c7d037bf304d6c582cd7f595b80372
Headers show
Series Fix UndefinedBehaviorSanitizer reported issues and enable it in CI. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Dumitru Ceara Jan. 24, 2022, 2:17 p.m. UTC
UB Sanitizer report:
  lib/stopwatch.c:119:22: runtime error: index 18446744073709551615 out of bounds for type 'long long unsigned int [50]'
      #0 0x698358 in calc_percentile lib/stopwatch.c:119
      #1 0x69ada1 in add_sample lib/stopwatch.c:231
      #2 0x69c086 in stopwatch_end_sample_protected lib/stopwatch.c:386
      #3 0x69c522 in stopwatch_thread lib/stopwatch.c:441
      #4 0x684bae in ovsthread_wrapper lib/ovs-thread.c:383
      #5 0x7f042838b298 in start_thread (/lib64/libpthread.so.0+0x9298)
      #6 0x7f04277f2352 in clone (/lib64/libc.so.6+0x100352)

Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
v3: Added Aaron's ack.
---
 lib/stopwatch.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Paolo Valerio Feb. 3, 2022, 7:02 p.m. UTC | #1
Dumitru Ceara <dceara@redhat.com> writes:

> UB Sanitizer report:
>   lib/stopwatch.c:119:22: runtime error: index 18446744073709551615 out of bounds for type 'long long unsigned int [50]'
>       #0 0x698358 in calc_percentile lib/stopwatch.c:119
>       #1 0x69ada1 in add_sample lib/stopwatch.c:231
>       #2 0x69c086 in stopwatch_end_sample_protected lib/stopwatch.c:386
>       #3 0x69c522 in stopwatch_thread lib/stopwatch.c:441
>       #4 0x684bae in ovsthread_wrapper lib/ovs-thread.c:383
>       #5 0x7f042838b298 in start_thread (/lib64/libpthread.so.0+0x9298)
>       #6 0x7f04277f2352 in clone (/lib64/libc.so.6+0x100352)
>
> Acked-by: Aaron Conole <aconole@redhat.com>
> Signed-off-by: Dumitru Ceara <dceara@redhat.com>
> ---

LGTM,

Acked-by: Paolo Valerio <pvalerio@redhat.com>
Ilya Maximets Feb. 15, 2022, 1:14 p.m. UTC | #2
On 2/3/22 20:02, Paolo Valerio wrote:
> Dumitru Ceara <dceara@redhat.com> writes:
> 
>> UB Sanitizer report:
>>   lib/stopwatch.c:119:22: runtime error: index 18446744073709551615 out of bounds for type 'long long unsigned int [50]'
>>       #0 0x698358 in calc_percentile lib/stopwatch.c:119
>>       #1 0x69ada1 in add_sample lib/stopwatch.c:231
>>       #2 0x69c086 in stopwatch_end_sample_protected lib/stopwatch.c:386
>>       #3 0x69c522 in stopwatch_thread lib/stopwatch.c:441
>>       #4 0x684bae in ovsthread_wrapper lib/ovs-thread.c:383
>>       #5 0x7f042838b298 in start_thread (/lib64/libpthread.so.0+0x9298)
>>       #6 0x7f04277f2352 in clone (/lib64/libc.so.6+0x100352)
>>
>> Acked-by: Aaron Conole <aconole@redhat.com>
>> Signed-off-by: Dumitru Ceara <dceara@redhat.com>
>> ---
> 
> LGTM,
> 
> Acked-by: Paolo Valerio <pvalerio@redhat.com>
> 

Thanks!  Applied and backported down to 2.13.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/stopwatch.c b/lib/stopwatch.c
index f5602163bc16..1c71df1a127e 100644
--- a/lib/stopwatch.c
+++ b/lib/stopwatch.c
@@ -114,7 +114,6 @@  static void
 calc_percentile(unsigned long long n_samples, struct percentile *pctl,
                 unsigned long long new_sample)
 {
-
     if (n_samples < P_SQUARE_MIN) {
         pctl->samples[n_samples - 1] = new_sample;
     }
@@ -228,13 +227,12 @@  add_sample(struct stopwatch *sw, unsigned long long new_sample)
         sw->min = new_sample;
     }
 
-    calc_percentile(sw->n_samples, &sw->pctl, new_sample);
-
     if (sw->n_samples++ == 0) {
         sw->short_term.average = sw->long_term.average = new_sample;
         return;
     }
 
+    calc_percentile(sw->n_samples, &sw->pctl, new_sample);
     calc_average(&sw->short_term, new_sample);
     calc_average(&sw->long_term, new_sample);
 }