Message-ID: <CAG_fn=UG3O-3_ik0TY_kstxzMVh4Z9noTP1cYfAiWvCnaXQ-6A@mail.gmail.com>
Date: Mon, 7 Jul 2025 11:47:43 +0200
From: Alexander Potapenko <glider@...gle.com>
To: Alejandro Colomar <alx@...nel.org>
Cc: linux-mm@...ck.org, linux-hardening@...r.kernel.org, 
	Kees Cook <kees@...nel.org>, Christopher Bazley <chris.bazley.wg14@...il.com>, 
	shadow <~hallyn/shadow@...ts.sr.ht>, linux-kernel@...r.kernel.org, 
	Andrew Morton <akpm@...ux-foundation.org>, kasan-dev@...glegroups.com, 
	Dmitry Vyukov <dvyukov@...gle.com>, Marco Elver <elver@...gle.com>, Christoph Lameter <cl@...ux.com>, 
	David Rientjes <rientjes@...gle.com>, Vlastimil Babka <vbabka@...e.cz>, 
	Roman Gushchin <roman.gushchin@...ux.dev>, Harry Yoo <harry.yoo@...cle.com>
Subject: Re: [RFC v1 1/3] vsprintf: Add [v]seprintf(), [v]stprintf()

On Sat, Jul 5, 2025 at 10:33 PM Alejandro Colomar <alx@...nel.org> wrote:
>
> seprintf()
> ==========
>
> seprintf() is a function similar to stpcpy(3) in the sense that it
> returns a pointer that is suitable for chaining to other copy
> operations.
>
> It takes a pointer to the end of the buffer as a sentinel for when to
> truncate, which unlike a size, doesn't need to be updated after every
> call.  This makes it much more ergonomic, avoiding manually calculating
> the size after each copy, which is error prone.
>
> It also makes error handling much easier, by reporting truncation with
> a null pointer, which is accepted and transparently passed down by
> subsequent seprintf() calls.  This results in only needing to report
> errors once after a chain of seprintf() calls, unlike snprintf(3), which
> requires checking after every call.
>
>         p = buf;
>         e = buf + countof(buf);
>         p = seprintf(p, e, foo);
>         p = seprintf(p, e, bar);
>         if (p == NULL)
>                 goto trunc;
>
> vs
>
>         len = 0;
>         size = countof(buf);
>         len += snprintf(buf + len, size - len, foo);
>         if (len >= size)
>                 goto trunc;
>
>         len += snprintf(buf + len, size - len, bar);
>         if (len >= size)
>                 goto trunc;
>
> And also better than scnprintf() calls:
>
>         len = 0;
>         size = countof(buf);
>         len += scnprintf(buf + len, size - len, foo);
>         len += scnprintf(buf + len, size - len, bar);
>         if (len >= size)
>                 goto trunc;
>
> It seems aparent that it's a more elegant approach to string catenation.
>
> stprintf()
> ==========
>
> stprintf() is a helper that is needed for implementing seprintf()
> --although it could be open-coded within vseprintf(), of course--, but
> it's also useful by itself.  It has the same interface properties as
> strscpy(): that is, it copies with truncation, and reports truncation
> with -E2BIG.  It would be useful to replace some calls to snprintf(3)
> and scnprintf() which don't need chaining, and where it's simpler to
> pass a size.
>
> It is better than plain snprintf(3), because it results in simpler error
> detection (it doesn't need a check >=countof(buf), but rather <0).
>
> Cc: Kees Cook <kees@...nel.org>
> Cc: Christopher Bazley <chris.bazley.wg14@...il.com>
> Signed-off-by: Alejandro Colomar <alx@...nel.org>
> ---
>  lib/vsprintf.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 109 insertions(+)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 01699852f30c..a3efacadb5e5 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -2892,6 +2892,37 @@ int vsnprintf(char *buf, size_t size, const char *fmt_str, va_list args)
>  }
>  EXPORT_SYMBOL(vsnprintf);
>
> +/**
> + * vstprintf - Format a string and place it in a buffer
> + * @buf: The buffer to place the result into
> + * @size: The size of the buffer, including the trailing null space
> + * @fmt: The format string to use
> + * @args: Arguments for the format string
> + *
> + * The return value is the length of the new string.
> + * If the string is truncated, the function returns -E2BIG.
> + *
> + * If you're not already dealing with a va_list consider using stprintf().
> + *
> + * See the vsnprintf() documentation for format string extensions over C99.
> + */
> +int vstprintf(char *buf, size_t size, const char *fmt, va_list args)
> +{
> +       int len;
> +
> +       len = vsnprintf(buf, size, fmt, args);
> +
> +       // It seems the kernel's vsnprintf() doesn't fail?
> +       //if (unlikely(len < 0))
> +       //      return -E2BIG;
> +
> +       if (unlikely(len >= size))
> +               return -E2BIG;
> +
> +       return len;
> +}
> +EXPORT_SYMBOL(vstprintf);
> +
>  /**
>   * vscnprintf - Format a string and place it in a buffer
>   * @buf: The buffer to place the result into
> @@ -2923,6 +2954,36 @@ int vscnprintf(char *buf, size_t size, const char *fmt, va_list args)
>  }
>  EXPORT_SYMBOL(vscnprintf);
>
> +/**
> + * vseprintf - Format a string and place it in a buffer
> + * @p: The buffer to place the result into
> + * @end: A pointer to one past the last character in the buffer
> + * @fmt: The format string to use
> + * @args: Arguments for the format string
> + *
> + * The return value is a pointer to the trailing '\0'.
> + * If @p is NULL, the function returns NULL.
> + * If the string is truncated, the function returns NULL.
> + *
> + * If you're not already dealing with a va_list consider using seprintf().
> + *
> + * See the vsnprintf() documentation for format string extensions over C99.
> + */
> +char *vseprintf(char *p, const char end[0], const char *fmt, va_list args)
> +{
> +       int len;
> +
> +       if (unlikely(p == NULL))
> +               return NULL;
> +
> +       len = vstprintf(p, end - p, fmt, args);

It's easy to imagine a situation in which `end` is calculated from the
user input and may overflow.
Maybe we can add a check for `end > p` to be on the safe side?

Powered by blists - more mailing lists