diff mbox series

[PATCH-for-5.2,v3] util/cutils: Fix Coverity array overrun in freq_to_str()

Message ID 20201101215755.2021421-1-f4bug@amsat.org
State Superseded
Headers show
Series [PATCH-for-5.2,v3] util/cutils: Fix Coverity array overrun in freq_to_str() | expand

Commit Message

Philippe Mathieu-Daudé Nov. 1, 2020, 9:57 p.m. UTC 
Fix Coverity CID 1435957:  Memory - illegal accesses (OVERRUN):

>>> Overrunning array "suffixes" of 7 8-byte elements at element
    index 7 (byte offset 63) using index "idx" (which evaluates to 7).

Note, the biggest input value freq_to_str() can accept is UINT64_MAX,
which is ~18.446 EHz, less than 1000 EHz.

Reported-by: Eduardo Habkost <ehabkost@redhat.com>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
v3: Follow Peter's suggestion
---
 util/cutils.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Peter Maydell Nov. 1, 2020, 10:01 p.m. UTC | #1 
On Sun, 1 Nov 2020 at 21:57, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>

> Fix Coverity CID 1435957:  Memory - illegal accesses (OVERRUN):

>

> >>> Overrunning array "suffixes" of 7 8-byte elements at element

>     index 7 (byte offset 63) using index "idx" (which evaluates to 7).

>

> Note, the biggest input value freq_to_str() can accept is UINT64_MAX,

> which is ~18.446 EHz, less than 1000 EHz.

>

> Reported-by: Eduardo Habkost <ehabkost@redhat.com>

> Suggested-by: Peter Maydell <peter.maydell@linaro.org>

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---

> v3: Follow Peter's suggestion

> ---

>  util/cutils.c | 3 ++-

>  1 file changed, 2 insertions(+), 1 deletion(-)

>

> diff --git a/util/cutils.c b/util/cutils.c

> index c395974fab4..2f869a843a5 100644

> --- a/util/cutils.c

> +++ b/util/cutils.c

> @@ -891,10 +891,11 @@ char *freq_to_str(uint64_t freq_hz)

>      double freq = freq_hz;

>      size_t idx = 0;

>

> -    while (freq >= 1000.0 && idx < ARRAY_SIZE(suffixes)) {

> +    while (freq >= 1000.0) {

>          freq /= 1000.0;

>          idx++;

>      }

> +    assert(idx < ARRAY_SIZE(suffixes));

>

>      return g_strdup_printf("%0.3g %sHz", freq, suffixes[idx]);

>  }

> --


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>


thanks
-- PMM
Eduardo Habkost Nov. 2, 2020, 1:45 p.m. UTC | #2 
On Sun, Nov 01, 2020 at 10:57:55PM +0100, Philippe Mathieu-Daudé wrote:
> Fix Coverity CID 1435957:  Memory - illegal accesses (OVERRUN):

> 

> >>> Overrunning array "suffixes" of 7 8-byte elements at element

>     index 7 (byte offset 63) using index "idx" (which evaluates to 7).

> 

> Note, the biggest input value freq_to_str() can accept is UINT64_MAX,

> which is ~18.446 EHz, less than 1000 EHz.

> 

> Reported-by: Eduardo Habkost <ehabkost@redhat.com>

> Suggested-by: Peter Maydell <peter.maydell@linaro.org>

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>


Thanks for taking the time to fix this!

-- 
Eduardo
Luc Michel Nov. 2, 2020, 1:50 p.m. UTC | #3 
On 22:57 Sun 01 Nov     , Philippe Mathieu-Daudé wrote:
> Fix Coverity CID 1435957:  Memory - illegal accesses (OVERRUN):

> 

> >>> Overrunning array "suffixes" of 7 8-byte elements at element

>     index 7 (byte offset 63) using index "idx" (which evaluates to 7).

> 

> Note, the biggest input value freq_to_str() can accept is UINT64_MAX,

> which is ~18.446 EHz, less than 1000 EHz.

> 

> Reported-by: Eduardo Habkost <ehabkost@redhat.com>

> Suggested-by: Peter Maydell <peter.maydell@linaro.org>

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


Reviewed-by: Luc Michel <luc@lmichel.fr>


> ---

> v3: Follow Peter's suggestion

> ---

>  util/cutils.c | 3 ++-

>  1 file changed, 2 insertions(+), 1 deletion(-)

> 

> diff --git a/util/cutils.c b/util/cutils.c

> index c395974fab4..2f869a843a5 100644

> --- a/util/cutils.c

> +++ b/util/cutils.c

> @@ -891,10 +891,11 @@ char *freq_to_str(uint64_t freq_hz)

>      double freq = freq_hz;

>      size_t idx = 0;

>  

> -    while (freq >= 1000.0 && idx < ARRAY_SIZE(suffixes)) {

> +    while (freq >= 1000.0) {

>          freq /= 1000.0;

>          idx++;

>      }

> +    assert(idx < ARRAY_SIZE(suffixes));

>  

>      return g_strdup_printf("%0.3g %sHz", freq, suffixes[idx]);

>  }

> -- 

> 2.26.2

> 


--
Peter Maydell Nov. 12, 2020, 3:55 p.m. UTC | #4 
On Sun, 1 Nov 2020 at 21:57, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>

> Fix Coverity CID 1435957:  Memory - illegal accesses (OVERRUN):

>

> >>> Overrunning array "suffixes" of 7 8-byte elements at element

>     index 7 (byte offset 63) using index "idx" (which evaluates to 7).

>

> Note, the biggest input value freq_to_str() can accept is UINT64_MAX,

> which is ~18.446 EHz, less than 1000 EHz.

>

> Reported-by: Eduardo Habkost <ehabkost@redhat.com>

> Suggested-by: Peter Maydell <peter.maydell@linaro.org>

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---

> v3: Follow Peter's suggestion


Since nobody else has picked this up I'll take it via
target-arm.next.

thanks
-- PMM
diff mbox series

Patch

diff --git a/util/cutils.c b/util/cutils.c
index c395974fab4..2f869a843a5 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -891,10 +891,11 @@  char *freq_to_str(uint64_t freq_hz)
     double freq = freq_hz;
     size_t idx = 0;
 
-    while (freq >= 1000.0 && idx < ARRAY_SIZE(suffixes)) {
+    while (freq >= 1000.0) {
         freq /= 1000.0;
         idx++;
     }
+    assert(idx < ARRAY_SIZE(suffixes));
 
     return g_strdup_printf("%0.3g %sHz", freq, suffixes[idx]);
 }