1. Patchew
  2. QEMU
  3. View series

[PATCH v4] i386: Fix GCC warning with snprintf when HAX is enabled

Julio Faracco posted 1 patch 5 years, 8 months ago
Test docker-quick@centos7 failed
Test FreeBSD failed
Test docker-mingw@fedora failed
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200302201320.6465-1-jcfaracco@gmail.com
target/i386/hax-posix.c   | 33 ++-------------------------------
target/i386/hax-windows.c | 33 ++-------------------------------
2 files changed, 4 insertions(+), 62 deletions(-)
[PATCH v4] i386: Fix GCC warning with snprintf when HAX is enabled
Posted by Julio Faracco 5 years, 8 months ago 
When HAX is enabled (--enable-hax), GCC 9.2.1 reports issues with
snprintf(). Replacing old snprintf() by g_strdup_printf() fixes the
problem with boundary checks of vm_id and vcpu_id and finally the
warnings produced by GCC.

For more details, one example of warning:
  CC      i386-softmmu/target/i386/hax-posix.o
qemu/target/i386/hax-posix.c: In function ‘hax_host_open_vm’:
qemu/target/i386/hax-posix.c:124:56: error: ‘%02d’ directive output may be
truncated writing between 2 and 11 bytes into a region of size 3
[-Werror=format-truncation=]
  124 |     snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
      |                                                        ^~~~
qemu/target/i386/hax-posix.c:124:41: note: directive argument in the range
[-2147483648, 64]
  124 |     snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
      |                                         ^~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/stdio.h:867,
                 from qemu/include/qemu/osdep.h:99,
                 from qemu/target/i386/hax-posix.c:14:
/usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output
between 17 and 26 bytes into a destination of size 17
   67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   68 |        __bos (__s), __fmt, __va_arg_pack ());
      |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
v1-v2: Add assert() as Richard Henderson suggested.
v2-v3: Fix code syntax alignment with vm_id and snprintf() function.
v3-v4: Replacing snprintf() by g_strdup_printf() from linux and windows.
---
---
 target/i386/hax-posix.c   | 33 ++-------------------------------
 target/i386/hax-windows.c | 33 ++-------------------------------
 2 files changed, 4 insertions(+), 62 deletions(-)

diff --git a/target/i386/hax-posix.c b/target/i386/hax-posix.c
index a5426a6dac..3bad89f133 100644
--- a/target/i386/hax-posix.c
+++ b/target/i386/hax-posix.c
@@ -108,41 +108,12 @@ int hax_mod_version(struct hax_state *hax, struct hax_module_version *version)
 
 static char *hax_vm_devfs_string(int vm_id)
 {
-    char *name;
-
-    if (vm_id > MAX_VM_ID) {
-        fprintf(stderr, "Too big VM id\n");
-        return NULL;
-    }
-
-#define HAX_VM_DEVFS "/dev/hax_vm/vmxx"
-    name = g_strdup(HAX_VM_DEVFS);
-    if (!name) {
-        return NULL;
-    }
-
-    snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
-    return name;
+    return g_strdup_printf("/dev/hax_vm/vm%02d", vm_id);
 }
 
 static char *hax_vcpu_devfs_string(int vm_id, int vcpu_id)
 {
-    char *name;
-
-    if (vm_id > MAX_VM_ID || vcpu_id > MAX_VCPU_ID) {
-        fprintf(stderr, "Too big vm id %x or vcpu id %x\n", vm_id, vcpu_id);
-        return NULL;
-    }
-
-#define HAX_VCPU_DEVFS "/dev/hax_vmxx/vcpuxx"
-    name = g_strdup(HAX_VCPU_DEVFS);
-    if (!name) {
-        return NULL;
-    }
-
-    snprintf(name, sizeof HAX_VCPU_DEVFS, "/dev/hax_vm%02d/vcpu%02d",
-             vm_id, vcpu_id);
-    return name;
+    return g_strdup_printf("/dev/hax_vm%02d/vcpu%02d", vm_id, vcpu_id);
 }
 
 int hax_host_create_vm(struct hax_state *hax, int *vmid)
diff --git a/target/i386/hax-windows.c b/target/i386/hax-windows.c
index 5729ad9b48..0ba488c468 100644
--- a/target/i386/hax-windows.c
+++ b/target/i386/hax-windows.c
@@ -185,41 +185,12 @@ int hax_mod_version(struct hax_state *hax, struct hax_module_version *version)
 
 static char *hax_vm_devfs_string(int vm_id)
 {
-    char *name;
-
-    if (vm_id > MAX_VM_ID) {
-        fprintf(stderr, "Too big VM id\n");
-        return NULL;
-    }
-
-#define HAX_VM_DEVFS "\\\\.\\hax_vmxx"
-    name = g_strdup(HAX_VM_DEVFS);
-    if (!name) {
-        return NULL;
-    }
-
-    snprintf(name, sizeof HAX_VM_DEVFS, "\\\\.\\hax_vm%02d", vm_id);
-    return name;
+    return g_strdup_printf("/dev/hax_vm/vm%02d", vm_id);
 }
 
 static char *hax_vcpu_devfs_string(int vm_id, int vcpu_id)
 {
-    char *name;
-
-    if (vm_id > MAX_VM_ID || vcpu_id > MAX_VCPU_ID) {
-        fprintf(stderr, "Too big vm id %x or vcpu id %x\n", vm_id, vcpu_id);
-        return NULL;
-    }
-
-#define HAX_VCPU_DEVFS "\\\\.\\hax_vmxx_vcpuxx"
-    name = g_strdup(HAX_VCPU_DEVFS);
-    if (!name) {
-        return NULL;
-    }
-
-    snprintf(name, sizeof HAX_VCPU_DEVFS, "\\\\.\\hax_vm%02d_vcpu%02d",
-             vm_id, vcpu_id);
-    return name;
+    return g_strdup_printf("/dev/hax_vm%02d/vcpu%02d", vm_id, vcpu_id);
 }
 
 int hax_host_create_vm(struct hax_state *hax, int *vmid)
-- 
2.24.1
Re: [PATCH v4] i386: Fix GCC warning with snprintf when HAX is enabled
Posted by Paolo Bonzini 5 years, 8 months ago 
On 02/03/20 21:13, Julio Faracco wrote:
> When HAX is enabled (--enable-hax), GCC 9.2.1 reports issues with
> snprintf(). Replacing old snprintf() by g_strdup_printf() fixes the
> problem with boundary checks of vm_id and vcpu_id and finally the
> warnings produced by GCC.
> 
> For more details, one example of warning:
>   CC      i386-softmmu/target/i386/hax-posix.o
> qemu/target/i386/hax-posix.c: In function ‘hax_host_open_vm’:
> qemu/target/i386/hax-posix.c:124:56: error: ‘%02d’ directive output may be
> truncated writing between 2 and 11 bytes into a region of size 3
> [-Werror=format-truncation=]
>   124 |     snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
>       |                                                        ^~~~
> qemu/target/i386/hax-posix.c:124:41: note: directive argument in the range
> [-2147483648, 64]
>   124 |     snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
>       |                                         ^~~~~~~~~~~~~~~~~~~~
> In file included from /usr/include/stdio.h:867,
>                  from qemu/include/qemu/osdep.h:99,
>                  from qemu/target/i386/hax-posix.c:14:
> /usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output
> between 17 and 26 bytes into a destination of size 17
>    67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    68 |        __bos (__s), __fmt, __va_arg_pack ());
>       |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
> v1-v2: Add assert() as Richard Henderson suggested.
> v2-v3: Fix code syntax alignment with vm_id and snprintf() function.
> v3-v4: Replacing snprintf() by g_strdup_printf() from linux and windows.
> ---
> ---
>  target/i386/hax-posix.c   | 33 ++-------------------------------
>  target/i386/hax-windows.c | 33 ++-------------------------------
>  2 files changed, 4 insertions(+), 62 deletions(-)
> 
> diff --git a/target/i386/hax-posix.c b/target/i386/hax-posix.c
> index a5426a6dac..3bad89f133 100644
> --- a/target/i386/hax-posix.c
> +++ b/target/i386/hax-posix.c
> @@ -108,41 +108,12 @@ int hax_mod_version(struct hax_state *hax, struct hax_module_version *version)
>  
>  static char *hax_vm_devfs_string(int vm_id)
>  {
> -    char *name;
> -
> -    if (vm_id > MAX_VM_ID) {
> -        fprintf(stderr, "Too big VM id\n");
> -        return NULL;
> -    }
> -
> -#define HAX_VM_DEVFS "/dev/hax_vm/vmxx"
> -    name = g_strdup(HAX_VM_DEVFS);
> -    if (!name) {
> -        return NULL;
> -    }
> -
> -    snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
> -    return name;
> +    return g_strdup_printf("/dev/hax_vm/vm%02d", vm_id);
>  }
>  
>  static char *hax_vcpu_devfs_string(int vm_id, int vcpu_id)
>  {
> -    char *name;
> -
> -    if (vm_id > MAX_VM_ID || vcpu_id > MAX_VCPU_ID) {
> -        fprintf(stderr, "Too big vm id %x or vcpu id %x\n", vm_id, vcpu_id);
> -        return NULL;
> -    }
> -
> -#define HAX_VCPU_DEVFS "/dev/hax_vmxx/vcpuxx"
> -    name = g_strdup(HAX_VCPU_DEVFS);
> -    if (!name) {
> -        return NULL;
> -    }
> -
> -    snprintf(name, sizeof HAX_VCPU_DEVFS, "/dev/hax_vm%02d/vcpu%02d",
> -             vm_id, vcpu_id);
> -    return name;
> +    return g_strdup_printf("/dev/hax_vm%02d/vcpu%02d", vm_id, vcpu_id);
>  }
>  
>  int hax_host_create_vm(struct hax_state *hax, int *vmid)
> diff --git a/target/i386/hax-windows.c b/target/i386/hax-windows.c
> index 5729ad9b48..0ba488c468 100644
> --- a/target/i386/hax-windows.c
> +++ b/target/i386/hax-windows.c
> @@ -185,41 +185,12 @@ int hax_mod_version(struct hax_state *hax, struct hax_module_version *version)
>  
>  static char *hax_vm_devfs_string(int vm_id)
>  {
> -    char *name;
> -
> -    if (vm_id > MAX_VM_ID) {
> -        fprintf(stderr, "Too big VM id\n");
> -        return NULL;
> -    }
> -
> -#define HAX_VM_DEVFS "\\\\.\\hax_vmxx"
> -    name = g_strdup(HAX_VM_DEVFS);
> -    if (!name) {
> -        return NULL;
> -    }
> -
> -    snprintf(name, sizeof HAX_VM_DEVFS, "\\\\.\\hax_vm%02d", vm_id);
> -    return name;
> +    return g_strdup_printf("/dev/hax_vm/vm%02d", vm_id);
>  }
>  
>  static char *hax_vcpu_devfs_string(int vm_id, int vcpu_id)
>  {
> -    char *name;
> -
> -    if (vm_id > MAX_VM_ID || vcpu_id > MAX_VCPU_ID) {
> -        fprintf(stderr, "Too big vm id %x or vcpu id %x\n", vm_id, vcpu_id);
> -        return NULL;
> -    }
> -
> -#define HAX_VCPU_DEVFS "\\\\.\\hax_vmxx_vcpuxx"
> -    name = g_strdup(HAX_VCPU_DEVFS);
> -    if (!name) {
> -        return NULL;
> -    }
> -
> -    snprintf(name, sizeof HAX_VCPU_DEVFS, "\\\\.\\hax_vm%02d_vcpu%02d",
> -             vm_id, vcpu_id);
> -    return name;
> +    return g_strdup_printf("/dev/hax_vm%02d/vcpu%02d", vm_id, vcpu_id);
>  }
>  
>  int hax_host_create_vm(struct hax_state *hax, int *vmid)
> 

Queued, thanks.

Paolo
Re: [PATCH v4] i386: Fix GCC warning with snprintf when HAX is enabled
Posted by Julio Faracco 5 years, 8 months ago 
Em seg., 2 de mar. de 2020 às 17:13, Julio Faracco
<jcfaracco@gmail.com> escreveu:
>
> When HAX is enabled (--enable-hax), GCC 9.2.1 reports issues with
> snprintf(). Replacing old snprintf() by g_strdup_printf() fixes the
> problem with boundary checks of vm_id and vcpu_id and finally the
> warnings produced by GCC.
>
> For more details, one example of warning:
>   CC      i386-softmmu/target/i386/hax-posix.o
> qemu/target/i386/hax-posix.c: In function ‘hax_host_open_vm’:
> qemu/target/i386/hax-posix.c:124:56: error: ‘%02d’ directive output may be
> truncated writing between 2 and 11 bytes into a region of size 3
> [-Werror=format-truncation=]
>   124 |     snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
>       |                                                        ^~~~
> qemu/target/i386/hax-posix.c:124:41: note: directive argument in the range
> [-2147483648, 64]
>   124 |     snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
>       |                                         ^~~~~~~~~~~~~~~~~~~~
> In file included from /usr/include/stdio.h:867,
>                  from qemu/include/qemu/osdep.h:99,
>                  from qemu/target/i386/hax-posix.c:14:
> /usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output
> between 17 and 26 bytes into a destination of size 17
>    67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    68 |        __bos (__s), __fmt, __va_arg_pack ());
>       |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
> v1-v2: Add assert() as Richard Henderson suggested.
> v2-v3: Fix code syntax alignment with vm_id and snprintf() function.
> v3-v4: Replacing snprintf() by g_strdup_printf() from linux and windows.
> ---
> ---
>  target/i386/hax-posix.c   | 33 ++-------------------------------
>  target/i386/hax-windows.c | 33 ++-------------------------------
>  2 files changed, 4 insertions(+), 62 deletions(-)
>
> diff --git a/target/i386/hax-posix.c b/target/i386/hax-posix.c
> index a5426a6dac..3bad89f133 100644
> --- a/target/i386/hax-posix.c
> +++ b/target/i386/hax-posix.c
> @@ -108,41 +108,12 @@ int hax_mod_version(struct hax_state *hax, struct hax_module_version *version)
>
>  static char *hax_vm_devfs_string(int vm_id)
>  {
> -    char *name;
> -
> -    if (vm_id > MAX_VM_ID) {
> -        fprintf(stderr, "Too big VM id\n");
> -        return NULL;
> -    }
> -
> -#define HAX_VM_DEVFS "/dev/hax_vm/vmxx"
> -    name = g_strdup(HAX_VM_DEVFS);
> -    if (!name) {
> -        return NULL;
> -    }
> -
> -    snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
> -    return name;
> +    return g_strdup_printf("/dev/hax_vm/vm%02d", vm_id);
>  }
>
>  static char *hax_vcpu_devfs_string(int vm_id, int vcpu_id)
>  {
> -    char *name;
> -
> -    if (vm_id > MAX_VM_ID || vcpu_id > MAX_VCPU_ID) {
> -        fprintf(stderr, "Too big vm id %x or vcpu id %x\n", vm_id, vcpu_id);
> -        return NULL;
> -    }

We probably need a V5 to remove definitions from hax-i386.h:
#define MAX_VM_ID 0x40
#define MAX_VCPU_ID 0x40

That's why I asked about IF last patch. ;-)

> -
> -#define HAX_VCPU_DEVFS "/dev/hax_vmxx/vcpuxx"
> -    name = g_strdup(HAX_VCPU_DEVFS);
> -    if (!name) {
> -        return NULL;
> -    }
> -
> -    snprintf(name, sizeof HAX_VCPU_DEVFS, "/dev/hax_vm%02d/vcpu%02d",
> -             vm_id, vcpu_id);
> -    return name;
> +    return g_strdup_printf("/dev/hax_vm%02d/vcpu%02d", vm_id, vcpu_id);
>  }
>
>  int hax_host_create_vm(struct hax_state *hax, int *vmid)
> diff --git a/target/i386/hax-windows.c b/target/i386/hax-windows.c
> index 5729ad9b48..0ba488c468 100644
> --- a/target/i386/hax-windows.c
> +++ b/target/i386/hax-windows.c
> @@ -185,41 +185,12 @@ int hax_mod_version(struct hax_state *hax, struct hax_module_version *version)
>
>  static char *hax_vm_devfs_string(int vm_id)
>  {
> -    char *name;
> -
> -    if (vm_id > MAX_VM_ID) {
> -        fprintf(stderr, "Too big VM id\n");
> -        return NULL;
> -    }
> -
> -#define HAX_VM_DEVFS "\\\\.\\hax_vmxx"
> -    name = g_strdup(HAX_VM_DEVFS);
> -    if (!name) {
> -        return NULL;
> -    }
> -
> -    snprintf(name, sizeof HAX_VM_DEVFS, "\\\\.\\hax_vm%02d", vm_id);
> -    return name;
> +    return g_strdup_printf("/dev/hax_vm/vm%02d", vm_id);
>  }
>
>  static char *hax_vcpu_devfs_string(int vm_id, int vcpu_id)
>  {
> -    char *name;
> -
> -    if (vm_id > MAX_VM_ID || vcpu_id > MAX_VCPU_ID) {
> -        fprintf(stderr, "Too big vm id %x or vcpu id %x\n", vm_id, vcpu_id);
> -        return NULL;
> -    }
> -
> -#define HAX_VCPU_DEVFS "\\\\.\\hax_vmxx_vcpuxx"
> -    name = g_strdup(HAX_VCPU_DEVFS);
> -    if (!name) {
> -        return NULL;
> -    }
> -
> -    snprintf(name, sizeof HAX_VCPU_DEVFS, "\\\\.\\hax_vm%02d_vcpu%02d",
> -             vm_id, vcpu_id);
> -    return name;
> +    return g_strdup_printf("/dev/hax_vm%02d/vcpu%02d", vm_id, vcpu_id);
>  }
>
>  int hax_host_create_vm(struct hax_state *hax, int *vmid)
> --
> 2.24.1
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.