hvmloader: Drop use of XENVER_extraversion

Commit Message

Andrew Cooper Feb. 11, 2020, 2:08 p.m. UTC

The printf() in init_hypercalls() only ends up in the hypervisor console log,
so extraversion really isn't interesting.

The SMBios table doesn't need extraversion, and removing it reduces the
ability for a guest to fingerprint the exact hypervisor it is running under.

Signed-off-by: Andrew Cooper <[e-mail]>
---
CC: Jan Beulich <[e-mail]>
CC: Wei Liu <[e-mail]>
CC: Roger Pau Monné <[e-mail]>
---
 tools/firmware/hvmloader/hvmloader.c |  4 +---
 tools/firmware/hvmloader/smbios.c    | 10 ----------
 2 files changed, 1 insertion(+), 13 deletions(-)

Comments

Jan Beulich Feb. 12, 2020, 9:44 a.m. UTC | #1

On 11.02.2020 15:08, Andrew Cooper wrote:
> The printf() in init_hypercalls() only ends up in the hypervisor console log,
> so extraversion really isn't interesting.
> 
> The SMBios table doesn't need extraversion, and removing it reduces the
> ability for a guest to fingerprint the exact hypervisor it is running under.

While I'm not entirely opposed, let's compare with bare hardware's
BIOSes / SMBIOS tables: Don't you see, just like I do, typically
very detailed BIOS etc version information (sometimes including
even something like build numbers) there? I can accept that excess
data in extraversion may go too far, but the minor version number
we put there by default is hardly of any concern, and may end up
being useful. The one argument against its usefulness is that it's
generally string of no-where standardized contents.

Personally I like Sergey's approach better, but I realize you
dislike it to a degree that, as it seems, you're not even willing
to have a reasonable dispute over it.

Jan

Patch

diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
index 598a226278..99c8841790 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -121,7 +121,6 @@  static void init_hypercalls(void)
     uint32_t eax, ebx, ecx, edx;
     unsigned long i;
     char signature[13];
-    xen_extraversion_t extraversion;
     uint32_t base;
 
     for ( base = 0x40000000; base < 0x40010000; base += 0x100 )
@@ -146,8 +145,7 @@  static void init_hypercalls(void)
 
     /* Print version information. */
     cpuid(base + 1, &eax, &ebx, &ecx, &edx);
-    hypercall_xen_version(XENVER_extraversion, extraversion);
-    printf("Detected Xen v%u.%u%s\n", eax >> 16, eax & 0xffff, extraversion);
+    printf("Detected Xen v%u.%u\n", eax >> 16, eax & 0xffff);
 }
 
 /* Replace possibly erroneous memory-size CMOS fields with correct values. */
diff --git a/tools/firmware/hvmloader/smbios.c b/tools/firmware/hvmloader/smbios.c
index 97a054e9e3..46ba1cb7b3 100644
--- a/tools/firmware/hvmloader/smbios.c
+++ b/tools/firmware/hvmloader/smbios.c
@@ -256,7 +256,6 @@  hvm_write_smbios_tables(
     xen_domain_handle_t uuid;
     uint16_t xen_major_version, xen_minor_version;
     uint32_t xen_version;
-    char xen_extra_version[XEN_EXTRAVERSION_LEN];
     /* guess conservatively on buffer length for Xen version string */
     char xen_version_str[80];
     /* temporary variables used to build up Xen version string */
@@ -274,8 +273,6 @@  hvm_write_smbios_tables(
     xen_major_version = (uint16_t) (xen_version >> 16);
     xen_minor_version = (uint16_t) xen_version;
 
-    hypercall_xen_version(XENVER_extraversion, xen_extra_version);
-
     /* build up human-readable Xen version string */
     p = xen_version_str;
     len = 0;
@@ -302,13 +299,6 @@  hvm_write_smbios_tables(
     strcpy(p, tmp);
     p += tmp_len;
 
-    tmp_len = strlen(xen_extra_version);
-    len += tmp_len;
-    if ( len >= sizeof(xen_version_str) )
-        goto error_out;
-    strcpy(p, xen_extra_version);
-    p += tmp_len;
-
     xen_version_str[sizeof(xen_version_str)-1] = '\0';
 
     /* scratch_start is a safe large memory area for scratch. */