[RFC v3 0/7] Add and use seprintf() instead of less ergonomic APIs

[Alejandro Colomar <alx@kernel.org>]

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