On 12/19/23 12:27, Egor Makrushin wrote:
> Multiplication results in integer overflow.
> Replace value of 6th agrument with ULLONG_MAX.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 04bd77a19f ("conf: Move and rename virDomainParseScaledValue()")
> Signed-off-by: Egor Makrushin <emakrushin@xxxxxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 58a985fc5d..871fd3a874 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8523,7 +8523,7 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt,
> unsigned long long bytes;
> if ((rc = virParseScaledValue("./pcihole64", NULL,
> ctxt, &bytes, 1024,
> - 1024ULL * ULONG_MAX, false)) < 0)
> + ULLONG_MAX, false)) < 0)
So IIUC, overflow happens when sizeof(ulong) == sizeof(ull). Now, the
reason there is 1024 * ULONG_MAX is because ..
> return NULL;
>
> if (rc == 1)
... down here, 'bytes' is divided by 1024 and stored in an ulong
variable. If we do the change you are suggesting then there is no
overflow up there, but there might be an overflow here, because at least
on one of my 32bit machines ulong is 4 bytes, and ulong long is 8 bytes.
A proper fix IMO would be to change the type of the variable
(def->opts.pciopts.pcihole64size) to ULL and then we can pass ULLONG_MAX
here. Alternatively, we may have some compile time evaluation of
maximums and decide whether we need the multiplication by 1024 or not.
Michal
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
