| Message ID | 20201029203850.434351-1-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [for-5.2] util/cutils: Fix bounds check at freq_to_str() | expand |
On 10/29/20 9:38 PM, Eduardo Habkost wrote: > Fix bounds check for idx at freq_to_str(), to actually ensure idx > never goes beyond the last element of the suffixes array. > > Reported-by: Coverity (CID 1435957: OVERRUN) > Suggested-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > util/cutils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/util/cutils.c b/util/cutils.c > index c395974fab..0d9261e1e5 100644 > --- a/util/cutils.c > +++ b/util/cutils.c > @@ -891,7 +891,7 @@ 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 && idx < ARRAY_SIZE(suffixes) - 1) { This was the first patch I wrote, but thought this wasn't the simplest way. Probably too tired. Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Thanks. > freq /= 1000.0; > idx++; > } >
On Thu, 29 Oct 2020 at 20:38, Eduardo Habkost <ehabkost@redhat.com> wrote: > > Fix bounds check for idx at freq_to_str(), to actually ensure idx > never goes beyond the last element of the suffixes array. > > Reported-by: Coverity (CID 1435957: OVERRUN) > Suggested-by: Peter Maydell <peter.maydell@linaro.org> Personally I preferred the other option (remove the idx check from the while loop and assert that idx is in bounds after the loop)... > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> thanks -- PMM
diff --git a/util/cutils.c b/util/cutils.c index c395974fab..0d9261e1e5 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -891,7 +891,7 @@ 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 && idx < ARRAY_SIZE(suffixes) - 1) { freq /= 1000.0; idx++; }
Fix bounds check for idx at freq_to_str(), to actually ensure idx never goes beyond the last element of the suffixes array. Reported-by: Coverity (CID 1435957: OVERRUN) Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- util/cutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)