[PATCH] net/tap-win32: Fix gcc 14 format truncation errors

Bernhard Beschow posted 1 patch 1 month, 2 weeks ago
There is a newer version of this series
net/tap-win32.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] net/tap-win32: Fix gcc 14 format truncation errors
Posted by Bernhard Beschow 1 month, 2 weeks ago
The patch fixes the following errors generated by GCC 14.2:

../src/net/tap-win32.c:343:19: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 176 [-Werror=format-truncation=]
  343 |              "%s\\%s\\Connection",
      |                   ^~
  344 |              NETWORK_CONNECTIONS_KEY, enum_name);
      |                                       ~~~~~~~~~

../src/net/tap-win32.c:341:9: note: 'snprintf' output between 92 and 347 bytes into a destination of size 256
  341 |         snprintf(connection_string,
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  342 |              sizeof(connection_string),
      |              ~~~~~~~~~~~~~~~~~~~~~~~~~~
  343 |              "%s\\%s\\Connection",
      |              ~~~~~~~~~~~~~~~~~~~~~
  344 |              NETWORK_CONNECTIONS_KEY, enum_name);
      |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

../src/net/tap-win32.c:242:58: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 178 [-Werror=format-truncation=]
  242 |         snprintf (unit_string, sizeof(unit_string), "%s\\%s",
      |                                                          ^~
  243 |                   ADAPTER_KEY, enum_name);
      |                                ~~~~~~~~~

../src/net/tap-win32.c:242:9: note: 'snprintf' output between 79 and 334 bytes into a destination of size 256
  242 |         snprintf (unit_string, sizeof(unit_string), "%s\\%s",
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  243 |                   ADAPTER_KEY, enum_name);
      |                   ~~~~~~~~~~~~~~~~~~~~~~~

../src/net/tap-win32.c:620:52: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 245 [-Werror=format-truncation=]
  620 |     snprintf (device_path, sizeof(device_path), "%s%s%s",
      |                                                    ^~
  621 |               USERMODEDEVICEDIR,
  622 |               device_guid,
      |               ~~~~~~~~~~~
../src/net/tap-win32.c:620:5: note: 'snprintf' output between 16 and 271 bytes into a destination of size 256
  620 |     snprintf (device_path, sizeof(device_path), "%s%s%s",
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  621 |               USERMODEDEVICEDIR,
      |               ~~~~~~~~~~~~~~~~~~
  622 |               device_guid,
      |               ~~~~~~~~~~~~
  623 |               TAPSUFFIX);
      |               ~~~~~~~~~~

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 net/tap-win32.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/tap-win32.c b/net/tap-win32.c
index 7edbd71633..4a4625af2b 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -214,7 +214,7 @@ static int is_tap_win32_dev(const char *guid)
 
     for (;;) {
         char enum_name[256];
-        char unit_string[256];
+        char unit_string[512];
         HKEY unit_key;
         char component_id_string[] = "ComponentId";
         char component_id[256];
@@ -315,7 +315,7 @@ static int get_device_guid(
     while (!stop)
     {
         char enum_name[256];
-        char connection_string[256];
+        char connection_string[512];
         HKEY connection_key;
         char name_data[256];
         DWORD name_type;
@@ -595,7 +595,7 @@ static void tap_win32_free_buffer(tap_win32_overlapped_t *overlapped,
 static int tap_win32_open(tap_win32_overlapped_t **phandle,
                           const char *preferred_name)
 {
-    char device_path[256];
+    char device_path[512];
     char device_guid[0x100];
     int rc;
     HANDLE handle;
-- 
2.46.2
Re: [PATCH] net/tap-win32: Fix gcc 14 format truncation errors
Posted by Michael Tokarev 1 month, 2 weeks ago
07.10.2024 13:13, Bernhard Beschow wrote:
> The patch fixes the following errors generated by GCC 14.2:
> 
> ../src/net/tap-win32.c:343:19: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 176 [-Werror=format-truncation=]
>    343 |              "%s\\%s\\Connection",
>        |                   ^~
>    344 |              NETWORK_CONNECTIONS_KEY, enum_name);
>        |                                       ~~~~~~~~~
...

>       for (;;) {
>           char enum_name[256];
> -        char unit_string[256];
> +        char unit_string[512];

Is it maybe better to use something like g_format_string() or asprintf() here?
Here and also in net/slirp.c

Thanks,

/mjt
Re: [PATCH] net/tap-win32: Fix gcc 14 format truncation errors
Posted by Bernhard Beschow 1 month, 2 weeks ago

Am 7. Oktober 2024 15:47:29 UTC schrieb Michael Tokarev <mjt@tls.msk.ru>:
>07.10.2024 13:13, Bernhard Beschow wrote:
>> The patch fixes the following errors generated by GCC 14.2:
>> 
>> ../src/net/tap-win32.c:343:19: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 176 [-Werror=format-truncation=]
>>    343 |              "%s\\%s\\Connection",
>>        |                   ^~
>>    344 |              NETWORK_CONNECTIONS_KEY, enum_name);
>>        |                                       ~~~~~~~~~
>...
>
>>       for (;;) {
>>           char enum_name[256];
>> -        char unit_string[256];
>> +        char unit_string[512];
>
>Is it maybe better to use something like g_format_string() or asprintf() here?

Will use g_autofree and g_strdup_printf() as Peter suggests.

>Here and also in net/slirp.c

There is a dedicated issue on Gitlab [1], so I'd keep the ball flat for now.

[1] https://gitlab.com/qemu-project/qemu/-/issues/2607

>
>Thanks,
>
>/mjt
Re: [PATCH] net/tap-win32: Fix gcc 14 format truncation errors
Posted by Peter Maydell 1 month, 2 weeks ago
On Mon, 7 Oct 2024 at 11:14, Bernhard Beschow <shentey@gmail.com> wrote:
>
> The patch fixes the following errors generated by GCC 14.2:
>
> ../src/net/tap-win32.c:343:19: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 176 [-Werror=format-truncation=]
>   343 |              "%s\\%s\\Connection",
>       |                   ^~
>   344 |              NETWORK_CONNECTIONS_KEY, enum_name);
>       |                                       ~~~~~~~~~
>
> ../src/net/tap-win32.c:341:9: note: 'snprintf' output between 92 and 347 bytes into a destination of size 256
>   341 |         snprintf(connection_string,
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   342 |              sizeof(connection_string),
>       |              ~~~~~~~~~~~~~~~~~~~~~~~~~~
>   343 |              "%s\\%s\\Connection",
>       |              ~~~~~~~~~~~~~~~~~~~~~
>   344 |              NETWORK_CONNECTIONS_KEY, enum_name);
>       |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> ../src/net/tap-win32.c:242:58: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 178 [-Werror=format-truncation=]
>   242 |         snprintf (unit_string, sizeof(unit_string), "%s\\%s",
>       |                                                          ^~
>   243 |                   ADAPTER_KEY, enum_name);
>       |                                ~~~~~~~~~
>
> ../src/net/tap-win32.c:242:9: note: 'snprintf' output between 79 and 334 bytes into a destination of size 256
>   242 |         snprintf (unit_string, sizeof(unit_string), "%s\\%s",
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   243 |                   ADAPTER_KEY, enum_name);
>       |                   ~~~~~~~~~~~~~~~~~~~~~~~
>
> ../src/net/tap-win32.c:620:52: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 245 [-Werror=format-truncation=]
>   620 |     snprintf (device_path, sizeof(device_path), "%s%s%s",
>       |                                                    ^~
>   621 |               USERMODEDEVICEDIR,
>   622 |               device_guid,
>       |               ~~~~~~~~~~~
> ../src/net/tap-win32.c:620:5: note: 'snprintf' output between 16 and 271 bytes into a destination of size 256
>   620 |     snprintf (device_path, sizeof(device_path), "%s%s%s",
>       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   621 |               USERMODEDEVICEDIR,
>       |               ~~~~~~~~~~~~~~~~~~
>   622 |               device_guid,
>       |               ~~~~~~~~~~~~
>   623 |               TAPSUFFIX);
>       |               ~~~~~~~~~~
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2607
Probably also worth
Cc: qemu-stable@nongnu.org

> ---
>  net/tap-win32.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/tap-win32.c b/net/tap-win32.c
> index 7edbd71633..4a4625af2b 100644
> --- a/net/tap-win32.c
> +++ b/net/tap-win32.c
> @@ -214,7 +214,7 @@ static int is_tap_win32_dev(const char *guid)
>
>      for (;;) {
>          char enum_name[256];
> -        char unit_string[256];
> +        char unit_string[512];
>          HKEY unit_key;
>          char component_id_string[] = "ComponentId";
>          char component_id[256];
> @@ -315,7 +315,7 @@ static int get_device_guid(
>      while (!stop)
>      {
>          char enum_name[256];
> -        char connection_string[256];
> +        char connection_string[512];
>          HKEY connection_key;
>          char name_data[256];
>          DWORD name_type;
> @@ -595,7 +595,7 @@ static void tap_win32_free_buffer(tap_win32_overlapped_t *overlapped,
>  static int tap_win32_open(tap_win32_overlapped_t **phandle,
>                            const char *preferred_name)
>  {
> -    char device_path[256];
> +    char device_path[512];
>      char device_guid[0x100];
>      int rc;
>      HANDLE handle;

Rather than just increasing the array sizes, I think we
should use g_autofree and g_strdup_printf(), like:

       g_autofree char* unit_string = NULL;

       [...]
       unit_string = g_strdup_printf("%s\\%s", ADAPTER_KEY, enum_name);

       (then no need for an explicit free)

All this only happens once at open, so we can certainly
happily take the cost of memory allocation, and it saves
us wondering about whether there's actually a maximum
limit on these string values. (Looking at the MS documentation,
I think registry keys have a limit of 255 chars, but
values are 16383 chars, so 512 would be more than needed
for a key and less than the theoretical maximum for a value.)

thanks
-- PMM