[Slirp] [PATCH 1/6] util: add slirp_fmt() helpers

[Philippe Mathieu-Daudé]

On 1/27/20 10:24 AM, marcandre.lureau at redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> 
> Various calls to snprintf() in libslirp assume that snprintf() returns
> "only" the number of bytes written (excluding terminating NUL).
> 
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.html#tag_16_159_04
> 
> "Upon successful completion, the snprintf() function shall return the
> number of bytes that would be written to s had n been sufficiently
> large excluding the terminating null byte."
> 
> Introduce slirp_fmt() that handles several pathological cases the
> way libslirp usually expect:
> 
> - treat error as fatal (instead of silently returning -1)
> 
> - fmt0() will always \0 end
> 
> - return the number of bytes actually written (instead of what would
> have been written, which would usually result in OOB later), including
> the ending \0 for fmt0()


Arf I started the same work... But only slirp_fmt(), no slirp_fmt0() so 
far. Good you already finished it.

> - warn if truncation happened (instead of ignoring)
> 
> Other less common cases can still be handled with strcpy/snprintf() etc.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> Reviewed-by: Samuel Thibault <samuel.thibault at ens-lyon.org>

In my patches I used:
Suggested-by: Laszlo Ersek <lersek at redhat.com>

> ---
>   src/util.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/util.h |  3 +++
>   2 files changed, 65 insertions(+)
> 
> diff --git a/src/util.c b/src/util.c
> index e596087..e3b6257 100644
> --- a/src/util.c
> +++ b/src/util.c
> @@ -364,3 +364,65 @@ void slirp_pstrcpy(char *buf, int buf_size, const char *str)
>       }
>       *q = '\0';
>   }
> +
> +static int slirp_vsnprintf(char *str, size_t size,
> +                           const char *format, va_list args)
> +{
> +    int rv = vsnprintf(str, size, format, args);
> +
> +    if (rv < 0) {
> +        g_error("vsnprintf() failed: %s", g_strerror(errno));

So here we abort() the program.
This is "dies on error".

> +    }
> +
> +    return rv;
> +}
> +
> +/*
> + * A snprintf()-like function that:
> + * - returns the number of bytes written (excluding optional \0-ending)
> + * - dies on error
> + * - warn on truncation

"warns"

> + */
> +int slirp_fmt(char *str, size_t size, const char *format, ...)
> +{
> +    va_list args;
> +    int rv;
> +
> +    va_start(args, format);
> +    rv = slirp_vsnprintf(str, size, format, args);
> +    va_end(args);
> +
> +    if (rv > size) {
> +        g_critical("vsnprintf() truncation");

Here we don't abort.
This is "warn on truncation".

IIRC Daniel or David raised the problem of using GLib functions that can 
be disabled at compile-time.

> +    }
> +
> +    return MIN(rv, size);
> +}
> +
> +/*
> + * A snprintf()-like function that:
> + * - always \0-end (unless size == 0)
> + * - returns the number of bytes actually written, including \0 ending
> + * - dies on error
> + * - warn on truncation

"warns"

> + */
> +int slirp_fmt0(char *str, size_t size, const char *format, ...)
> +{
> +    va_list args;
> +    int rv;
> +
> +    va_start(args, format);
> +    rv = slirp_vsnprintf(str, size, format, args);
> +    va_end(args);
> +
> +    if (rv >= size) {
> +        g_critical("vsnprintf() truncation");
> +        if (size > 0)
> +            str[size - 1] = '\0';
> +        rv = size;
> +    } else {
> +        rv += 1; /* include \0 */
> +    }
> +
> +    return rv;
> +}
> diff --git a/src/util.h b/src/util.h
> index e9c3073..5530c46 100644
> --- a/src/util.h
> +++ b/src/util.h
> @@ -181,4 +181,7 @@ static inline int slirp_socket_set_fast_reuse(int fd)
>   
>   void slirp_pstrcpy(char *buf, int buf_size, const char *str);
>   
> +int slirp_fmt(char *str, size_t size, const char *format, ...);
> +int slirp_fmt0(char *str, size_t size, const char *format, ...);
> +
>   #endif
>