[libvirt] [PATCH] esx: Fix nodeGetInfo so cpu model fits inside nodeinfo->model

Marcos Paulo de Souza posted 1 patch 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180812110519.12052-1-marcos.souza.org@gmail.com
Test syntax-check passed
src/esx/esx_driver.c | 2 ++
1 file changed, 2 insertions(+)
[libvirt] [PATCH] esx: Fix nodeGetInfo so cpu model fits inside nodeinfo->model
Posted by Marcos Paulo de Souza 5 years, 10 months ago
Commit 6c0d0210cbcd5d647f0d882c07f077d444bc707d changed the behavior of
virStr*cpy* functions, so now the nodeGetInfo call fails. Version 4.1.0
(default for Fedora 28) works:

Model: Intel Core i7-4500U CPU @ 1.80G

Current master tries to write "Intel Core i7-4500U CPU @ 1.80GHz", but
the string is bigger than nodeinfo->model (which is a char[32]). So this
patch "cuts" the string, and presents the same output from 4.1.0.

Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
---

 When testing both qemu and lxc drivers, both return "x86_64", as both call
 virCapabilitiesGetNodeInfo in order the get all info the "node", but they are a
 Linux machine.

 When it comes to ESX, it returns a string from the ESX server, which is same
 info that I can find in /proc/cpuinfo of my machine (but without the () parts):
 model name      : Intel(R) Core(TM) i7-4500U CPU @ 1.80GHz

 The question is: should libvirt increase the model size, or just find a better
 way for ESX to present this info?

 src/esx/esx_driver.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index c2154799fa..03a84d7630 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -1317,6 +1317,8 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo)
                 ++ptr;
             }
 
+            /* Make sure the string fits in mode */
+            dynamicProperty->val->string[sizeof(nodeinfo->model) - 1] = '\0';
             if (virStrcpyStatic(nodeinfo->model, dynamicProperty->val->string) < 0) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("CPU Model %s too long for destination"),
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Fix nodeGetInfo so cpu model fits inside nodeinfo->model
Posted by Michal Prívozník 5 years, 10 months ago
On 08/12/2018 01:05 PM, Marcos Paulo de Souza wrote:
> Commit 6c0d0210cbcd5d647f0d882c07f077d444bc707d changed the behavior of
> virStr*cpy* functions, so now the nodeGetInfo call fails. Version 4.1.0
> (default for Fedora 28) works:
> 
> Model: Intel Core i7-4500U CPU @ 1.80G
> 
> Current master tries to write "Intel Core i7-4500U CPU @ 1.80GHz", but
> the string is bigger than nodeinfo->model (which is a char[32]). So this
> patch "cuts" the string, and presents the same output from 4.1.0.
> 
> Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> ---
> 
>  When testing both qemu and lxc drivers, both return "x86_64", as both call
>  virCapabilitiesGetNodeInfo in order the get all info the "node", but they are a
>  Linux machine.
> 
>  When it comes to ESX, it returns a string from the ESX server, which is same
>  info that I can find in /proc/cpuinfo of my machine (but without the () parts):
>  model name      : Intel(R) Core(TM) i7-4500U CPU @ 1.80GHz
> 
>  The question is: should libvirt increase the model size, or just find a better
>  way for ESX to present this info?

We can't do that. It would break ABI. Any app that is linking with
libvirt right now expects to see a 32 bytes long string. If we change
that to anything bigger we would overwrite the app's memory. Needless to
say, XDR would be broken too.

> 
>  src/esx/esx_driver.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index c2154799fa..03a84d7630 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
> @@ -1317,6 +1317,8 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo)
>                  ++ptr;
>              }
>  
> +            /* Make sure the string fits in mode */
> +            dynamicProperty->val->string[sizeof(nodeinfo->model) - 1] = '\0';
>              if (virStrcpyStatic(nodeinfo->model, dynamicProperty->val->string) < 0) {
>                  virReportError(VIR_ERR_INTERNAL_ERROR,
>                                 _("CPU Model %s too long for destination"),
> 

This in fact is the only way to fix the issue IMO.

ACKed and pushed.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list