[Libguestfs] [PATCH 2/2] GCC 7: Allocate sufficient space for sprintf output.

[Daniel P. Berrange]

On Tue, Feb 14, 2017 at 03:02:14PM +0000, Richard W.M. Jones wrote:
> GCC 7.0.1 can determine if there is likely to be sufficient space in
> the output buffer when using sprintf/snprintf, based on the format
> string.
> 
> The errors were all either of this form:
> 
> bindtests.c:717:29: error: '%zu' directive output may be truncated writing between 1 and 19 bytes into a region of size 16 [-Werror=format-truncation=]
>      snprintf (strs[i], 16, "%zu", i);
>                              ^~~
> bindtests.c:717:28: note: directive argument in the range [0, 2305843009213693951]
>      snprintf (strs[i], 16, "%zu", i);
>                              ^~~~~
> 
> or this form:
> 
> sync.c: In function 'fsync_devices':
> sync.c:108:50: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 251 [-Werror=format-truncation=]
>        snprintf (dev_path, sizeof dev_path, "/dev/%s", d->d_name);
>                                                   ^~
> 
> Fixed by converting these into dynamic allocation, or making the
> output buffer larger, whichever was easier.
> ---
>  cat/filesystems.c      |  2 +-
>  daemon/9p.c            | 10 +++++++---
>  daemon/debug.c         | 12 ++++++++++--
>  daemon/devsparts.c     | 16 ++++++++++++----
>  daemon/sync.c          |  7 +++++--
>  generator/bindtests.ml | 12 ++++++------
>  6 files changed, 41 insertions(+), 18 deletions(-)
> 
> diff --git a/cat/filesystems.c b/cat/filesystems.c
> index 1036c6f..4264b0f 100644
> --- a/cat/filesystems.c
> +++ b/cat/filesystems.c
> @@ -866,7 +866,7 @@ write_row (const char *name, const char *type,
>    size_t len = 0;
>    char hum[LONGEST_HUMAN_READABLE];
>    char num[256];
> -  char mbr_id_str[3];
> +  char mbr_id_str[9];

[snip]

> @@ -258,8 +258,8 @@ fill_lvm_pv (guestfs_h *g, struct guestfs_lvm_pv *pv, size_t i)
>               pr "  }\n";
>               pr "  strs = safe_malloc (g, (n+1) * sizeof (char *));\n";
>               pr "  for (i = 0; i < n; ++i) {\n";
> -             pr "    strs[i] = safe_malloc (g, 16);\n";
> -             pr "    snprintf (strs[i], 16, \"%%zu\", i);\n";
> +             pr "    strs[i] = safe_malloc (g, 20);\n";
> +             pr "    snprintf (strs[i], 20, \"%%zu\", i);\n";
>               pr "  }\n";
>               pr "  strs[n] = NULL;\n";
>               pr "  return strs;\n"
> @@ -290,10 +290,10 @@ fill_lvm_pv (guestfs_h *g, struct guestfs_lvm_pv *pv, size_t i)
>               pr "  }\n";
>               pr "  strs = safe_malloc (g, (n*2+1) * sizeof (*strs));\n";
>               pr "  for (i = 0; i < n; ++i) {\n";
> -             pr "    strs[i*2] = safe_malloc (g, 16);\n";
> -             pr "    strs[i*2+1] = safe_malloc (g, 16);\n";
> -             pr "    snprintf (strs[i*2], 16, \"%%zu\", i);\n";
> -             pr "    snprintf (strs[i*2+1], 16, \"%%zu\", i);\n";
> +             pr "    strs[i*2] = safe_malloc (g, 20);\n";
> +             pr "    strs[i*2+1] = safe_malloc (g, 20);\n";
> +             pr "    snprintf (strs[i*2], 20, \"%%zu\", i);\n";
> +             pr "    snprintf (strs[i*2+1], 20, \"%%zu\", i);\n";
>               pr "  }\n";
>               pr "  strs[n*2] = NULL;\n";
>               pr "  return strs;\n"

FWIW, for cases where you really do want to print an integer into a
fixed length buffer, then gnulib has a useful macro to let you
automatically determine the correct buffer size at compile time.

    size_t someval;
    char buf[INT_BUFSIZE_BOUND(someval)]

    snprintf(buf, ARRAY_CARDINALITY(buf), "%zu", someval)


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|