include/linux/array_size.h | 6 ++ include/linux/sprintf.h | 8 +++ include/linux/stackdepot.h | 13 +++++ include/linux/stacktrace.h | 3 + kernel/stacktrace.c | 28 ++++++++++ lib/stackdepot.c | 12 ++++ lib/vsprintf.c | 109 +++++++++++++++++++++++++++++++++++++ mm/backing-dev.c | 2 +- mm/cma.c | 4 +- mm/cma_debug.c | 2 +- mm/hugetlb.c | 3 +- mm/hugetlb_cgroup.c | 2 +- mm/hugetlb_cma.c | 2 +- mm/kasan/report.c | 3 +- mm/kfence/kfence_test.c | 28 +++++----- mm/kmsan/kmsan_test.c | 6 +- mm/memblock.c | 4 +- mm/mempolicy.c | 18 +++--- mm/page_owner.c | 32 ++++++----- mm/percpu.c | 2 +- mm/shrinker_debug.c | 2 +- mm/slub.c | 5 +- mm/zswap.c | 2 +- 23 files changed, 238 insertions(+), 58 deletions(-)
Hi,
In this v3:
- I've added Fixes: tags for all commits that introduced issues being
fixed in this patch set. I've also added the people who signed or
reviewed those patches to CC.
- I've fixed a typo in a comment.
- I've also added a STPRINTF() macro and used it to remove explicit
uses of sizeof().
Now, only 5 calls to snprintf(3) remain under mm/:
$ grep -rnI nprint mm/
mm/hugetlb_cgroup.c:674: snprintf(buf, size, "%luGB", hsize / SZ_1G);
mm/hugetlb_cgroup.c:676: snprintf(buf, size, "%luMB", hsize / SZ_1M);
mm/hugetlb_cgroup.c:678: snprintf(buf, size, "%luKB", hsize / SZ_1K);
mm/kfence/report.c:75: int len = scnprintf(buf, sizeof(buf), "%ps", (void *)stack_entries[skipnr]);
mm/kmsan/report.c:42: len = scnprintf(buf, sizeof(buf), "%ps",
The first three are fine. The remaining two, I'd like someone to check
if they should be replaced by one of these wrappers. I had doubts about
it, and would need someone understanding that code to check them.
Mainly, do we really want to ignore truncation?
The questions from v1 still are in the air.
I've written an analysis of snprintf(3), why it's dangerous, and how
these APIs address that, and will present it as a proposal for
standardization of these APIs in ISO C2y. I'll send that as a reply to
this message in a moment, as I believe it will be interesting for
linux-hardening@.
Have a lovely night!
Alex
Alejandro Colomar (7):
vsprintf: Add [v]seprintf(), [v]stprintf()
stacktrace, stackdepot: Add seprintf()-like variants of functions
mm: Use seprintf() instead of less ergonomic APIs
array_size.h: Add ENDOF()
mm: Fix benign off-by-one bugs
sprintf: Add [V]STPRINTF()
mm: Use [V]STPRINTF() to avoid specifying the array size
include/linux/array_size.h | 6 ++
include/linux/sprintf.h | 8 +++
include/linux/stackdepot.h | 13 +++++
include/linux/stacktrace.h | 3 +
kernel/stacktrace.c | 28 ++++++++++
lib/stackdepot.c | 12 ++++
lib/vsprintf.c | 109 +++++++++++++++++++++++++++++++++++++
mm/backing-dev.c | 2 +-
mm/cma.c | 4 +-
mm/cma_debug.c | 2 +-
mm/hugetlb.c | 3 +-
mm/hugetlb_cgroup.c | 2 +-
mm/hugetlb_cma.c | 2 +-
mm/kasan/report.c | 3 +-
mm/kfence/kfence_test.c | 28 +++++-----
mm/kmsan/kmsan_test.c | 6 +-
mm/memblock.c | 4 +-
mm/mempolicy.c | 18 +++---
mm/page_owner.c | 32 ++++++-----
mm/percpu.c | 2 +-
mm/shrinker_debug.c | 2 +-
mm/slub.c | 5 +-
mm/zswap.c | 2 +-
23 files changed, 238 insertions(+), 58 deletions(-)
Range-diff against v2:
1: 64334f0b94d6 = 1: 64334f0b94d6 vsprintf: Add [v]seprintf(), [v]stprintf()
2: 9c140de9842d = 2: 9c140de9842d stacktrace, stackdepot: Add seprintf()-like variants of functions
3: e3271b5f2ad9 ! 3: 033bf00f1fcf mm: Use seprintf() instead of less ergonomic APIs
@@ Commit message
Again, the 'p += snprintf()' anti-pattern. This is UB, and by
using seprintf() we've fixed the bug.
+ Fixes: f99e12b21b84 (2021-07-30; "kfence: add function to mask address bits")
+ [alx: that commit introduced dead code]
+ Fixes: af649773fb25 (2024-07-17; "mm/numa_balancing: teach mpol_to_str about the balancing mode")
+ [alx: that commit added p+=snprintf() calls, which are UB]
+ Fixes: 2291990ab36b (2008-04-28; "mempolicy: clean-up mpol-to-str() mempolicy formatting")
+ [alx: that commit changed p+=sprintf() into p+=snprintf(), which is still UB]
+ Fixes: 948927ee9e4f (2013-11-13; "mm, mempolicy: make mpol_to_str robust and always succeed")
+ [alx: that commit changes old code into p+=snprintf(), which is still UB]
+ [alx: that commit also produced dead code by leaving the last 'p+=...']
+ Fixes: d65360f22406 (2022-09-26; "mm/slub: clean up create_unique_id()")
+ [alx: that commit changed p+=sprintf() into p+=snprintf(), which is still UB]
Cc: Kees Cook <kees@kernel.org>
Cc: Christopher Bazley <chris.bazley.wg14@gmail.com>
+ Cc: Sven Schnelle <svens@linux.ibm.com>
+ Cc: Marco Elver <elver@google.com>
+ Cc: Heiko Carstens <hca@linux.ibm.com>
+ Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
+ Cc: "Huang, Ying" <ying.huang@intel.com>
+ Cc: Andrew Morton <akpm@linux-foundation.org>
+ Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
+ Cc: Linus Torvalds <torvalds@linux-foundation.org>
+ Cc: David Rientjes <rientjes@google.com>
+ Cc: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
+ Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>
+ Cc: Chao Yu <chao.yu@oppo.com>
+ Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
## mm/kfence/kfence_test.c ##
4: 5331d286ceca ! 4: d8bd0e1d308b array_size.h: Add ENDOF()
@@ include/linux/array_size.h
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
+/**
-+ * ENDOF - get a pointer to one past the last element in array @arr
-+ * @arr: array
++ * ENDOF - get a pointer to one past the last element in array @a
++ * @a: array
+ */
+#define ENDOF(a) (a + ARRAY_SIZE(a))
+
5: 08cfdd2bf779 ! 5: 740755c1a888 mm: Fix benign off-by-one bugs
@@ Commit message
'end' --that is, at most the terminating null byte will be written at
'end-1'--.
+ Fixes: bc8fbc5f305a (2021-02-26; "kfence: add test suite")
+ Fixes: 8ed691b02ade (2022-10-03; "kmsan: add tests for KMSAN")
Cc: Kees Cook <kees@kernel.org>
Cc: Christopher Bazley <chris.bazley.wg14@gmail.com>
+ Cc: Alexander Potapenko <glider@google.com>
+ Cc: Marco Elver <elver@google.com>
+ Cc: Dmitry Vyukov <dvyukov@google.com>
+ Cc: Alexander Potapenko <glider@google.com>
+ Cc: Jann Horn <jannh@google.com>
+ Cc: Andrew Morton <akpm@linux-foundation.org>
+ Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
## mm/kfence/kfence_test.c ##
-: ------------ > 6: 44d05559398c sprintf: Add [V]STPRINTF()
-: ------------ > 7: d0e95db3c80a mm: Use [V]STPRINTF() to avoid specifying the array size
--
2.50.0
On Mon, Jul 07, 2025 at 07:06:06AM +0200, Alejandro Colomar wrote:
> I've written an analysis of snprintf(3), why it's dangerous, and how
> these APIs address that, and will present it as a proposal for
> standardization of these APIs in ISO C2y. I'll send that as a reply to
> this message in a moment, as I believe it will be interesting for
> linux-hardening@.
Hi,
Here is the proposal for ISO C2y (see below). I'll also send it to the
C Committee for discussion.
Have a lovely night!
Alex
---
Name
alx-0049r1 - add seprintf()
Principles
- Codify existing practice to address evident deficiencies.
- Enable secure programming
Category
Standardize existing libc APIs
Author
Alejandro Colomar <alx@kernel.org>
Cc: Christopher Bazley <chris.bazley.wg14@gmail.com>
History
<https://www.alejandro-colomar.es/src/alx/alx/wg14/alx-0049.git/>
r0 (2025-07-06):
- Initial draft.
r1 (2025-07-06):
- wfix.
- tfix.
- Expand on the off-by-one bugs.
- Note that ignoring truncation is not valid most of the time.
Rationale
snprintf(3) is very difficult to chain for writing parts of a
string in separate calls, such as in a loop.
Let's start from the obvious sprintf(3) code (sprintf(3) will
not prevent overflow, but let's take it as a baseline from which
programmers start thinking):
p = buf;
for (...)
p += sprintf(p, ...);
Then, programmers will start thinking about preventing buffer
overflows. Programmers sometimes will naively add some buffer
size information and use snprintf(3):
p = buf;
size = countof(buf);
for (...)
p += snprintf(p, size - (p - buf), ...);
if (p >= buf + size) // or worse, (p > buf + size - 1)
goto fail;
(Except for minor differences, this kind of code can be found
everywhere. Here are a couple of examples:
<https://elixir.bootlin.com/linux/v6.14/source/mm/slub.c#L7231>
<https://elixir.bootlin.com/linux/v6.14/source/mm/mempolicy.c#L3369>.)
This has several issues, starting with the difficulty of getting
the second argument right. Sometimes, programmers will be too
confused, and slap a -1 there just to be safe.
p = buf;
size = countof(buf);
for (...)
p += snprintf(p, size - (p - buf) - 1, ...);
if (p >= buf + size -1)
goto fail;
(Except for minor differences, this kind of code can be found
everywhere. Here are a couple of examples:
<https://elixir.bootlin.com/linux/v6.14/source/mm/kfence/kfence_test.c#L113>
<https://elixir.bootlin.com/linux/v6.14/source/mm/kmsan/kmsan_test.c#L108>.)
Programmers will sometimes hold a pointer to one past the last
element in the array. This is a wise choice, as that pointer is
constant throughout the lifetime of the object. Then,
programmers might end up with something like this:
p = buf;
e = buf + countof(buf);
for (...)
p += snprintf(p, e - p, ...);
if (p >= end)
goto fail;
This is certainly much cleaner. Now a programmer might focus on
the fact that this can overflow the pointer. An easy approach
would be to make sure that the function never returns more than
the remaining size. That is, one could implement something like
this scnprintf() --name chosen to match the Linux kernel API of
the same name--. For the sake of simplicity, let's ignore
multiple evaluation of arguments.
#define scnprintf(s, size, ...) \
({ \
int len_; \
len_ = snprintf(s, size, __VA_ARGS__); \
if (len_ == -1) \
len_ = 0; \
if (len_ >= size) \
len_ = size - 1; \
\
len_; \
})
p = buf;
e = buf + countof(buf);
for (...)
p += scnprintf(p, e - p, ...);
(Except for minor differences, this kind of code can be found
everywhere. Here's an example:
<https://elixir.bootlin.com/linux/v6.14/source/mm/kfence/kfence_test.c#L131>.)
Now the programmer got rid of pointer overflow. However, they
now have silent truncation that cannot be detected. In some
cases this may seem good enough. However, often it's not. And
anyway, some code remains using snprintf(3) to be able to detect
truncation.
Moreover, this kind of code ignores the fact that vsnprintf(3)
can fail internally, in which case there's not even a truncated
string. In the kernel, they're fine, because their internal
vsnprintf() doesn't seem to ever fail, so they can always rely
on the truncated string. This is not reliable in projects that
rely on the libc vsnprintf(3).
For the code that needs to detect truncation, a programmer might
choose a different path. It would keep using snprintf(3), but
would use a temporary length variable instead of the pointer.
p = buf;
e = buf + countof(buf);
for (...) {
len = snprintf(p, e - p, ...);
if (len == -1)
goto fail;
if (len >= e - p)
goto fail;
p += len;
}
This is naturally error-prone. A colleague of mine --which is an
excellent programmer, to be clear--, had a bug even after
knowing about it and having tried to fix it. That shows how
hard it is to write this correctly:
<https://github.com/nginx/unit/pull/734#discussion_r1043963527>
In a similar fashion, the strlcpy(3) manual page from OpenBSD
documents a similar issue when chaining calls to strlcpy(3)
--which was designed with semantics equivalent to snprintf(3),
except for not formatting the string--:
| char *dir, *file, pname[MAXPATHLEN];
| size_t n;
|
| ...
|
| n = strlcpy(pname, dir, sizeof(pname));
| if (n >= sizeof(pname))
| goto toolong;
| if (strlcpy(pname + n, file, sizeof(pname) - n) >= sizeof(pname) - n)
| goto toolong;
|
| However, one may question the validity of such optimiza‐
| tions, as they defeat the whole purpose of strlcpy() and
| strlcat(). As a matter of fact, the first version of
| this manual page got it wrong.
Finally, a programmer might realize that while this is error-
prone, this is indeed the right thing to do. There's no way to
avoid it. One could then think of encapsulating this into an
API that at least would make it easy to write. Then, one might
wonder what the right parameters are for such an API. The only
immutable thing in the loop is 'e'. And apart from that, one
needs to know where to write, which is 'p'. Let's start with
those, and try to keep all the other information (size, len)
without escaping the API. Again, let's ignore multiple-
evaluation issues in this macro for the sake of simplicity.
#define foo(p, e, ...) \
({ \
int len_ = snprintf(p, e - p, __VA_ARGS__); \
if (len_ == -1) \
p = NULL; \
else if (len_ >= e - p) \
p = NULL; \
else \
p += len_; \
p;
})
p = buf;
e = buf + countof(buf);
for (...) {
p = foo(p, e, ...);
if (p == NULL)
goto fail;
}
We've advanced a lot. We got rid of the buffer overflow; we
also got rid of the error-prone code at call site. However, one
might think that checking for truncation after every call is
cumbersome. Indeed, it is possible to slightly tweak the
internals of foo() to propagate errors from previous calls.
#define seprintf(p, e, ...) \
({ \
if (p != NULL) { \
int len_; \
\
len_ = snprintf(p, e - p, __VA_ARGS__); \
if (len_ == -1) \
p = NULL; \
else if (len_ >= e - p) \
p = NULL; \
else \
p += len_; \
} \
p; \
})
p = buf;
e = buf + countof(buf);
for (...)
p = seprintf(p, e, ...);
if (p == NULL)
goto fail;
By propagating an input null pointer directly to the output of
the API, which I've called seprintf() --the 'e' refers to the
'end' pointer, which is the key in this API--, we've allowed
ignoring null pointers until after the very last call. If we
compare our resulting code to the sprintf(3)-based baseline, we
got --perhaps unsurprisingly-- something quite close to it:
p = buf;
for (...)
p += sprintf(p, ...);
vs
p = buf;
e = buf + countof(buf);
for (...)
p = seprintf(p, e, ...);
if (p == NULL)
goto fail;
And the seprintf() version is safe against both truncation and
buffer overflow.
Some important details of the API are:
- When 'p' is NULL, the API must preserve errno. This is
important to be able to determine the cause of the error
after all the chained calls, even when the error occurred in
some call in the middle of the chain.
- When truncation occurs, a distinct errno value must be used,
to signal the programmer that at least the string is reliable
to be used as a null-terminated string. The error code
chosen is E2BIG, for compatibility with strscpy(), a Linux
kernel internal API with which this API shares many features
in common.
- When a hard error (an internal snprintf(3) error) occurs, an
error code different than E2BIG must be used. It is
important to set errno, because if an implementation would
chose to return NULL without setting errno, an old value of
E2BIG could lead the programmer to believe the string was
successfully written (and truncated), and read it with
nefast consequences.
Prior art
This API is implemented in the shadow-utils project.
Plan9 designed something quite close, which they call
seprint(2). The parameters are the same --the right choice--,
but they got the semantics for corner cases wrong. Ironically,
the existing Plan9 code I've seen seems to expect the semantics
that I chose, regardless of the actual semantics of the Plan9
API. This is --I suspect--, because my semantics are actually
the intuitive semantics that one would naively guess of an API
with these parameters and return value.
I've implemented this API for the Linux kernel, and found and
fixed an amazing amount of bugs and other questionable code in
just the first handful of files that I inspected.
<https://lore.kernel.org/linux-hardening/cover.1751747518.git.alx@kernel.org/T/#t>
<https://lore.kernel.org/linux-hardening/cover.1751823326.git.alx@kernel.org/T/#t>
Future directions
The 'e = buf + _Countof(buf)' construct is something I've found
to be quite common. It would be interesting to have an
_Endof operator that would return a pointer to one past the last
element of an array. It would require an array operand, just
like _Countof. If an _Endof operator is deemed too cumbersome
for implementation, an endof() standard macro that expands to
the obvious implementation with _Countof could be okay.
This operator (or operator-like macro) would prevent off-by-one
bugs when calculating the end sentinel value, such as those
shown above (with links to Linux kernel real bugs).
Proposed wording
Based on N3550.
7.24.6 Input/output <stdio.h> :: Formatted input/output functions
## New section after 7.24.6.6 ("The snprintf function"):
+7.24.6.6+1 The <b>seprintf</b> function
+
+Synopsis
+1 #include <stdio.h>
+ char *seprintf(char *restrict p, const char end[0], const char *restrict format, ...);
+
+Description
+2 The <b>$0</b> function
+ is equivalent to <b>fprintf</b>,
+ except that the output is written into an array
+ (specified by argument <tt>p</tt>)
+ rather than a stream.
+ If <tt>p</tt> is a null pointer,
+ nothing is written,
+ and the function returns a null pointer.
+ Otherwise,
+ <tt>end</tt> shall compare greater than <tt>p</tt>;
+ the function writes at most
+ <tt>end - p - 1</tt> non-null characters,
+ the remaining output characters are discarded,
+ and a null character is written
+ at the end of the characters
+ actually written to the array.
+ If copying takes place between objects that overlap,
+ the behavior is undefined.
+
+Returns
+3 The <b>$0</b> function returns
+ a pointer to the terminating null character
+ if the output was written
+ without discarding any characters.
+
+4
+ If <tt>p</tt> is a null pointer,
+ a null pointer is returned,
+ and <b>errno</b> is not modified.
+
+5
+ If any characters are discarded,
+ a null pointer is returned,
+ and the value of the macro <b>E2BIG</b>
+ is stored in <b>errno</b>.
+
+6
+ If an error occurred,
+ a null pointer is returned,
+ and an implementation-defined non-zero value
+ is stored in <b>errno</b>.
## New section after 7.24.6.13 ("The vsnprintf function"):
+7.24.6.13+1 The <b>vseprintf</b> function
+
+Synopsis
+1 #include <stdio.h>
+ char *vseprintf(char *restrict p, const char end[0], const char *restrict format, va_list arg);
+
+Description
+2 The <b>$0</b> function
+ is equivalent to
+ <b>seprintf</b>,
+ with the varying argument list replaced by <tt>arg</tt>.
+
+3
+ The <tt>va_list</tt> argument to this function
+ shall have been initialized by the <b>va_start</b> macro
+ (and possibly subsequent <b>va_arg</b> invocations).
+ This function does not invoke the <b>va_end</b> macro.343)
7.33.2 Formatted wide character input/output functions
## New section after 7.33.2.4 ("The swprintf function"):
+7.33.2.4+1 The <b>sewprintf</b> function
+
+Synopsis
+1 #include <wchar.h>
+ wchar_t *sewprintf(wchar_t *restrict p, const wchar_t end[0], const wchar_t *restrict format, ...);
+
+Description
+2 The <b>$0</b> function
+ is equivalent to
+ <b>seprintf</b>,
+ except that it handles wide strings.
## New section after 7.33.2.8 ("The vswprintf function"):
+7.33.2.8+1 The <b>vsewprintf</b> function
+
+Synopsis
+1 #include <wchar.h>
+ wchar_t *vsewprintf(wchar_t *restrict p, const wchar_t end[0], const wchar_t *restrict format, va_list arg);
+
+Description
+2 The <b>$0</b> function
+ is equivalent to
+ <b>sewprintf</b>,
+ with the varying argument list replaced by <tt>arg</tt>.
+
+3
+ The <tt>va_list</tt> argument to this function
+ shall have been initialized by the <b>va_start</b> macro
+ (and possibly subsequent <b>va_arg</b> invocations).
+ This function does not invoke the <b>va_end</b> macro.407)
--
<https://www.alejandro-colomar.es/>
© 2016 - 2025 Red Hat, Inc.