diff mbox series

i386: Fix GCC warning with snprintf when HAX is enabled

Message ID 20200301163523.13581-1-jcfaracco@gmail.com
State New
Headers show
Series i386: Fix GCC warning with snprintf when HAX is enabled | expand

Commit Message

Julio Faracco March 1, 2020, 4:35 p.m. UTC
When HAX is enabled (--enable-hax), GCC 9.2.1 reports issues with
snprintf(). This commit is checking if snprintf returns an error. This
is a simple way to avoid this warnings.

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>
---
 target/i386/hax-posix.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Richard Henderson March 1, 2020, 6:36 p.m. UTC | #1
On 3/1/20 8:35 AM, Julio Faracco wrote:
> When HAX is enabled (--enable-hax), GCC 9.2.1 reports issues with
> snprintf(). This commit is checking if snprintf returns an error. This
> is a simple way to avoid this warnings.
> 
> 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]


The code bounds vm_id from the top (64) but not the bottom.  The compiler has
(correctly) determined that negative values will overflow your buffer.

I suggest either an assert vs negative values as a local change, or to change
the type of vm_id (universally, if possible) to an unsigned type.


r~
diff mbox series

Patch

diff --git a/target/i386/hax-posix.c b/target/i386/hax-posix.c
index a5426a6dac..7437d54b44 100644
--- a/target/i386/hax-posix.c
+++ b/target/i386/hax-posix.c
@@ -121,7 +121,11 @@  static char *hax_vm_devfs_string(int vm_id)
         return NULL;
     }
 
-    snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
+    if (snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d",
+                 vm_id) < 0) {
+        return NULL;
+    }
+
     return name;
 }
 
@@ -140,8 +144,11 @@  static char *hax_vcpu_devfs_string(int vm_id, int vcpu_id)
         return NULL;
     }
 
-    snprintf(name, sizeof HAX_VCPU_DEVFS, "/dev/hax_vm%02d/vcpu%02d",
-             vm_id, vcpu_id);
+    if (snprintf(name, sizeof HAX_VCPU_DEVFS, "/dev/hax_vm%02d/vcpu%02d",
+                 vm_id, vcpu_id) < 0) {
+        return NULL;
+    }
+
     return name;
 }