[Libguestfs] [PATCH common v2] utils: Fix usage of strerror_r

[Laszlo Ersek]

On 12/09/21 10:05, Richard W.M. Jones wrote:
>   $ ./run guestfish -vx add-drive foo "readonly:true"
>   libguestfs: trace: set_pgroup true
>   libguestfs: trace: set_pgroup = 0
>   libguestfs: trace: add_drive "foo" "readonly:true"
>   libguestfs: error: foo:
>   libguestfs: trace: add_drive = -1 (error)
>   libguestfs: trace: close
>   libguestfs: closing guestfs handle 0x55c0bacf12a0 (state 0)
> 
> Note the "error: foo: " line.  After hexdumping this I found that it
> is actually printing random rubbish after the colon.  It turns out
> that strerror_r was not updating the buffer and so we were printing
> random bytes from the stack.
> 
> strerror_r is hard to use and worse still there are two incompatible
> variants in glibc.  Close reading of the GNU variant that we are using
> shows that it doesn't always use the buffer passed in, which explains
> what was happening here.
> 
> Provide an easier to use wrapper around strerror_r and try to cope
> with the two variants using _GNU_SOURCE.
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2030396
> Reported-by: Masayoshi Mizuma <m.mizuma at jp.fujitsu.com>
> Tested-by: Masayoshi Mizuma <m.mizuma at jp.fujitsu.com>
> ---
>  utils/guestfs-utils.h |  1 +
>  utils/utils.c         | 25 +++++++++++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/utils/guestfs-utils.h b/utils/guestfs-utils.h
> index b5f5f3ca4..b1d923e0e 100644
> --- a/utils/guestfs-utils.h
> +++ b/utils/guestfs-utils.h
> @@ -67,6 +67,7 @@ extern int guestfs_int_is_lnk (int64_t mode);
>  extern int guestfs_int_is_sock (int64_t mode);
>  extern char *guestfs_int_full_path (const char *dir, const char *name);
>  extern void guestfs_int_hexdump (const void *data, size_t len, FILE *fp);
> +const char *guestfs_int_strerror (int errnum, char *buf, size_t buflen);
>  
>  /* Not all language bindings know how to deal with Pointer arguments.
>   * Those that don't will use this macro which complains noisily and
> diff --git a/utils/utils.c b/utils/utils.c
> index 6f4108732..70e55cb2e 100644
> --- a/utils/utils.c
> +++ b/utils/utils.c
> @@ -640,3 +640,28 @@ guestfs_int_hexdump (const void *data, size_t len, FILE *fp)
>      fprintf (fp, "|\n");
>    }
>  }
> +
> +/**
> + * Thread-safe strerror_r.
> + *
> + * This is a wrapper around the two variants of L<strerror_r(3)>
> + * in glibc since it is hard to use correctly (RHBZ#2030396).
> + *
> + * The buffer passed in should be large enough to store the
> + * error message (256 chars at least) and should be non-static.
> + * Note that the buffer might not be used, use the return value.
> + */
> +const char *
> +guestfs_int_strerror (int errnum, char *buf, size_t buflen)
> +{
> +#ifdef _GNU_SOURCE
> +  /* GNU strerror_r */
> +  return strerror_r (errnum, buf, buflen);
> +#else
> +  /* XSI-compliant strerror_r */
> +  int err = strerror_r (errnum, buf, buflen);
> +  if (err > 0)
> +    snprintf (buf, buflen, "error number %d", errnum);
> +  return buf;
> +#endif
> +}
> 

Reviewed-by: Laszlo Ersek <lersek at redhat.com>