Message-ID: <nx6vj5qqcgkts4pmefzux3ee4kuumwyjh6vlwsdltf56ayq33e@kyf25zkic2rk>
Date: Sat, 5 Jul 2025 22:40:49 +0200
From: Alejandro Colomar <alx@...nel.org>
To: linux-mm@...ck.org, linux-hardening@...r.kernel.org
Cc: 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>, 
	Alexander Potapenko <glider@...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 05, 2025 at 10:33:49PM +0200, Alejandro Colomar 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;

Oops, this error handling is incorrect, as scnprintf() doesn't report
truncation.  I should have compared

	p = buf;
	e = buf + countof(buf);
	p = seprintf(p, e, foo);
	p = seprintf(p, e, bar);

vs

	len = 0;
	size = countof(buf);
	len += scnprintf(buf + len, size - len, foo);
	len += scnprintf(buf + len, size - len, bar);

> 
> 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);
> +	if (unlikely(len < 0))
> +		return NULL;
> +
> +	return p + len;
> +}
> +EXPORT_SYMBOL(vseprintf);
> +
>  /**
>   * snprintf - Format a string and place it in a buffer
>   * @buf: The buffer to place the result into
> @@ -2950,6 +3011,30 @@ int snprintf(char *buf, size_t size, const char *fmt, ...)
>  }
>  EXPORT_SYMBOL(snprintf);
>  
> +/**
> + * stprintf - 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
> + * @...: Arguments for the format string
> + *
> + * The return value is the length of the new string.
> + * If the string is truncated, the function returns -E2BIG.
> + */
> +
> +int stprintf(char *buf, size_t size, const char *fmt, ...)
> +{
> +	va_list args;
> +	int len;
> +
> +	va_start(args, fmt);
> +	len = vstprintf(buf, size, fmt, args);
> +	va_end(args);
> +
> +	return len;
> +}
> +EXPORT_SYMBOL(stprintf);
> +
>  /**
>   * scnprintf - Format a string and place it in a buffer
>   * @buf: The buffer to place the result into
> @@ -2974,6 +3059,30 @@ int scnprintf(char *buf, size_t size, const char *fmt, ...)
>  }
>  EXPORT_SYMBOL(scnprintf);
>  
> +/**
> + * seprintf - 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
> + * @...: Arguments for the format string
> + *
> + * The return value is a pointer to the trailing '\0'.
> + * If @buf is NULL, the function returns NULL.
> + * If the string is truncated, the function returns NULL.
> + */
> +
> +char *seprintf(char *p, const char end[0], const char *fmt, ...)
> +{
> +	va_list args;
> +
> +	va_start(args, fmt);
> +	p = vseprintf(p, end, fmt, args);
> +	va_end(args);
> +
> +	return p;
> +}
> +EXPORT_SYMBOL(seprintf);
> +
>  /**
>   * vsprintf - Format a string and place it in a buffer
>   * @buf: The buffer to place the result into
> -- 
> 2.50.0
> 

-- 
<https://www.alejandro-colomar.es/>

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists