5:27 a.m.
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(a)astralinux.ru>
---
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)
return NULL;
if (rc == 1)
--
2.30.2
8:24 a.m.
On Tue, Dec 19, 2023 at 14:27:11 +0300, 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()")
The multiplication was there before this patch, since that's just moving
the code. If you want to use the 'Fixes' tag make sure to find the
commit that actually added the problem.
I can drop this tag if you want or you can find the proper commit.
Signed-off-by: Egor Makrushin <emakrushin(a)astralinux.ru>
---
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)
return NULL;
if (rc == 1)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
7:10 p.m.
19/12/23 17:24, Peter Krempa пишет:
On Tue, Dec 19, 2023 at 14:27:11 +0300, 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()")
The multiplication was there before this patch, since that's just moving
the code. If you want to use the 'Fixes' tag make sure to find the
commit that actually added the problem.
I can drop this tag if you want or you can find the proper commit.
Thanks for the
remark. I think it would be better to remove this tag,
which I will do in the next version of the patch.
> Signed-off-by: Egor Makrushin <emakrushin(a)astralinux.ru>
> ---
> 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)
> return NULL;
>
> if (rc == 1)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
Egor Makrushin
8:48 a.m.
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(a)astralinux.ru>
---
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
372
days inactive
373
days old
3 comments
3 participants
participants (3)
-
Egor Makrushin -
Michal Prívozník -
Peter Krempa