linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Martin Uecker <ma.uecker@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Alejandro Colomar <alx@kernel.org>
Cc: linux-mm@kvack.org, linux-hardening@vger.kernel.org,
	Kees Cook <kees@kernel.org>,
	Christopher Bazley <chris.bazley.wg14@gmail.com>,
	shadow <~hallyn/shadow@lists.sr.ht>,
	linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	kasan-dev@googlegroups.com, Dmitry Vyukov <dvyukov@google.com>,
	Alexander Potapenko <glider@google.com>,
	Marco Elver <elver@google.com>, Christoph Lameter <cl@linux.com>,
	David Rientjes <rientjes@google.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Harry Yoo <harry.yoo@oracle.com>,
	Andrew Clayton <andrew@digital-domain.net>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	 Michal Hocko <mhocko@suse.com>,
	Al Viro <viro@zeniv.linux.org.uk>, Sam James <sam@gentoo.org>,
	Andrew Pinski <pinskia@gmail.com>
Subject: Re: [RFC v5 6/7] sprintf: Add [v]sprintf_array()
Date: Fri, 11 Jul 2025 08:05:38 +0200	[thread overview]
Message-ID: <28c8689c7976b4755c0b5c2937326b0a3627ebf6.camel@gmail.com> (raw)
In-Reply-To: <CAHk-=wiNJQ6dVU8t7oM0sFpSqxyK8JZQXV5NGx7h+AE0PY4kag@mail.gmail.com>

Am Donnerstag, dem 10.07.2025 um 14:58 -0700 schrieb Linus Torvalds:
> On Thu, 10 Jul 2025 at 14:31, Alejandro Colomar <alx@kernel.org> wrote:
> > 
> > These macros are essentially the same as the 2-argument version of
> > strscpy(), but with a formatted string, and returning a pointer to the
> > terminating '\0' (or NULL, on error).
> 
> No.
> 
> Stop this garbage.
> 
> You took my suggestion, and then you messed it up.
> 
> Your version of sprintf_array() is broken. It evaluates 'a' twice.
> Because unlike ARRAY_SIZE(), your broken ENDOF() macro evaluates the
> argument.
> 
> And you did it for no reason I can see. You said that you wanted to
> return the end of the resulting string, but the fact is, not a single
> user seems to care, and honestly, I think it would be wrong to care.
> The size of the result is likely the more useful thing, or you could
> even make these 'void' or something.
> 
> But instead you made the macro be dangerous to use.
> 
> This kind of churn is WRONG. It _looks_ like a cleanup that doesn't
> change anything, but then it has subtle bugs that will come and bite
> us later because you did things wrong.
> 
> I'm NAK'ing all of this. This is BAD. Cleanup patches had better be
> fundamentally correct, not introduce broken "helpers" that will make
> for really subtle bugs.
> 
> Maybe nobody ever ends up having that first argument with a side
> effect. MAYBE. It's still very very wrong.
> 
>                 Linus

What I am puzzled about is that - if you revise your string APIs -,
you do not directly go for a safe abstraction that combines length
and pointer and instead keep using these fragile 80s-style string
functions and open-coded pointer and size computations that everybody
gets wrong all the time.

String handling could also look like this:


https://godbolt.org/z/dqGz9b4sM

and be completely bounds safe.

(Note that those function abort() on allocation failure, but this
is an unfinished demo and also not for kernel use. Also I need to
rewrite this using string views.)


Martin





  parent reply	other threads:[~2025-07-11  6:05 UTC|newest]

Thread overview: 98+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-05 20:33 [RFC v1 0/3] Add and use seprintf() instead of less ergonomic APIs Alejandro Colomar
2025-07-05 20:33 ` [RFC v1 1/3] vsprintf: Add [v]seprintf(), [v]stprintf() Alejandro Colomar
2025-07-05 20:40   ` Alejandro Colomar
2025-07-07  9:47   ` Alexander Potapenko
2025-07-07 14:59     ` Alejandro Colomar
2025-07-05 20:33 ` [RFC v1 2/3] stacktrace, stackdepot: Add seprintf()-like variants of functions Alejandro Colomar
2025-07-05 20:33 ` [RFC v1 3/3] mm: Use seprintf() instead of less ergonomic APIs Alejandro Colomar
2025-07-05 21:54   ` Alejandro Colomar
2025-07-06 17:37 ` [RFC v2 0/5] Add and use " Alejandro Colomar
2025-07-06 17:37   ` [RFC v2 1/5] vsprintf: Add [v]seprintf(), [v]stprintf() Alejandro Colomar
2025-07-06 17:37   ` [RFC v2 2/5] stacktrace, stackdepot: Add seprintf()-like variants of functions Alejandro Colomar
2025-07-06 17:37   ` [RFC v2 3/5] mm: Use seprintf() instead of less ergonomic APIs Alejandro Colomar
2025-07-06 17:37   ` [RFC v2 4/5] array_size.h: Add ENDOF() Alejandro Colomar
2025-07-06 17:37   ` [RFC v2 5/5] mm: Fix benign off-by-one bugs Alejandro Colomar
2025-07-07  5:06   ` [RFC v3 0/7] Add and use seprintf() instead of less ergonomic APIs Alejandro Colomar
2025-07-07  5:06     ` [RFC v3 1/7] vsprintf: Add [v]seprintf(), [v]stprintf() Alejandro Colomar
2025-07-07  5:06     ` [RFC v3 2/7] stacktrace, stackdepot: Add seprintf()-like variants of functions Alejandro Colomar
2025-07-07  5:06     ` [RFC v3 3/7] mm: Use seprintf() instead of less ergonomic APIs Alejandro Colomar
2025-07-07  7:44       ` Marco Elver
2025-07-07 14:39         ` Alejandro Colomar
2025-07-07 14:58           ` Marco Elver
2025-07-07 18:51             ` Alejandro Colomar
2025-07-07 19:08               ` Marco Elver
2025-07-07 20:53                 ` Alejandro Colomar
2025-07-07 19:17       ` Linus Torvalds
2025-07-07 19:35         ` Al Viro
2025-07-07 20:46           ` Linus Torvalds
2025-07-07 20:29         ` Alejandro Colomar
2025-07-07 20:49           ` Linus Torvalds
2025-07-07 21:05             ` Alejandro Colomar
2025-07-07 21:26               ` Alejandro Colomar
2025-07-07 22:17                 ` Linus Torvalds
2025-07-08  2:20                   ` Alejandro Colomar
2025-07-12 20:58         ` Christopher Bazley
2025-07-14  7:57           ` Christopher Bazley
2025-07-07  5:06     ` [RFC v3 4/7] array_size.h: Add ENDOF() Alejandro Colomar
2025-07-07  5:06     ` [RFC v3 5/7] mm: Fix benign off-by-one bugs Alejandro Colomar
2025-07-07  7:46       ` Marco Elver
2025-07-07  7:53         ` Michal Hocko
2025-07-07 14:42           ` Alejandro Colomar
2025-07-07 15:12             ` Michal Hocko
2025-07-07 15:29               ` Alejandro Colomar
2025-07-07  5:06     ` [RFC v3 6/7] sprintf: Add [V]STPRINTF() Alejandro Colomar
2025-07-07  5:06     ` [RFC v3 7/7] mm: Use [V]STPRINTF() to avoid specifying the array size Alejandro Colomar
2025-07-07  5:11     ` [RFC v3 0/7] Add and use seprintf() instead of less ergonomic APIs Alejandro Colomar
2025-07-10  2:47   ` [RFC v4 0/7] Add and use sprintf_end() " Alejandro Colomar
2025-07-10  2:47     ` alx-0049r2 - add seprintf() Alejandro Colomar
2025-07-10  2:48     ` [RFC v4 1/7] vsprintf: Add [v]sprintf_end() Alejandro Colomar
2025-07-10  2:48     ` [RFC v4 2/7] stacktrace, stackdepot: Add sprintf_end()-like variants of functions Alejandro Colomar
2025-07-10  2:48     ` [RFC v4 3/7] mm: Use sprintf_end() instead of less ergonomic APIs Alejandro Colomar
2025-07-10  2:48     ` [RFC v4 4/7] array_size.h: Add ENDOF() Alejandro Colomar
2025-07-10  2:48     ` [RFC v4 5/7] mm: Fix benign off-by-one bugs Alejandro Colomar
2025-07-10  2:48     ` [RFC v4 6/7] sprintf: Add [V]SPRINTF_END() Alejandro Colomar
2025-07-10 15:52       ` Linus Torvalds
2025-07-10 18:30         ` Alejandro Colomar
2025-07-10 21:21           ` Alejandro Colomar
2025-07-10 22:08             ` Linus Torvalds
2025-07-10  2:49     ` [RFC v4 7/7] mm: Use [V]SPRINTF_END() to avoid specifying the array size Alejandro Colomar
2025-07-10 21:30   ` [RFC v5 0/7] Add and use sprintf_{end,array}() instead of less ergonomic APIs Alejandro Colomar
2025-07-10 21:30     ` [RFC v5 1/7] vsprintf: Add [v]sprintf_end() Alejandro Colomar
2025-07-10 21:30     ` [RFC v5 2/7] stacktrace, stackdepot: Add sprintf_end()-like variants of functions Alejandro Colomar
2025-07-10 21:30     ` [RFC v5 3/7] mm: Use sprintf_end() instead of less ergonomic APIs Alejandro Colomar
2025-07-10 21:31     ` [RFC v5 4/7] array_size.h: Add ENDOF() Alejandro Colomar
2025-07-10 21:31     ` [RFC v5 5/7] mm: Fix benign off-by-one bugs Alejandro Colomar
2025-07-10 21:31     ` [RFC v5 6/7] sprintf: Add [v]sprintf_array() Alejandro Colomar
2025-07-10 21:58       ` Linus Torvalds
2025-07-10 23:23         ` Alejandro Colomar
2025-07-10 23:24           ` Alejandro Colomar
2025-07-11  0:19           ` Alejandro Colomar
2025-07-11 17:43           ` David Laight
2025-07-11 19:17             ` Alejandro Colomar
2025-07-11 19:21               ` Alejandro Colomar
2025-07-11  6:05         ` Martin Uecker [this message]
2025-07-11  6:19           ` Martin Uecker
2025-07-11 17:45           ` David Laight
2025-07-11 17:58             ` Linus Torvalds
2025-07-11 19:24               ` Matthew Wilcox
2025-07-15  5:19               ` Kees Cook
2025-07-15  6:24                 ` Martin Uecker
2025-07-17 23:44                   ` Kees Cook
2025-07-15  7:08                 ` Alejandro Colomar
2025-07-17 23:47                   ` Kees Cook
2025-07-18  0:56                     ` Alejandro Colomar
2025-07-11 18:01             ` Martin Uecker
2025-07-10 21:31     ` [RFC v5 7/7] mm: Use [v]sprintf_array() to avoid specifying the array size Alejandro Colomar
2025-07-11  1:56   ` [RFC v6 0/8] Add and use sprintf_{end,trunc,array}() instead of less ergonomic APIs Alejandro Colomar
2025-07-11  1:56     ` [RFC v6 1/8] vsprintf: Add [v]sprintf_trunc() Alejandro Colomar
2025-07-11  1:56     ` [RFC v6 2/8] vsprintf: Add [v]sprintf_end() Alejandro Colomar
2025-07-11  1:56     ` [RFC v6 3/8] sprintf: Add [v]sprintf_array() Alejandro Colomar
2025-07-11  1:56     ` [RFC v6 4/8] stacktrace, stackdepot: Add sprintf_end()-like variants of functions Alejandro Colomar
2025-07-11  1:57     ` [RFC v6 5/8] mm: Use sprintf_end() instead of less ergonomic APIs Alejandro Colomar
2025-07-11  1:57     ` [RFC v6 6/8] array_size.h: Add ENDOF() Alejandro Colomar
2025-07-11  1:57     ` [RFC v6 7/8] mm: Fix benign off-by-one bugs Alejandro Colomar
2025-07-11  1:57     ` [RFC v6 8/8] mm: Use [v]sprintf_array() to avoid specifying the array size Alejandro Colomar
2025-07-08  6:43 ` [RFC v1 0/3] Add and use seprintf() instead of less ergonomic APIs Rasmus Villemoes
2025-07-08 11:36   ` Alejandro Colomar
2025-07-08 13:51     ` Rasmus Villemoes
2025-07-08 16:14       ` Alejandro Colomar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=28c8689c7976b4755c0b5c2937326b0a3627ebf6.camel@gmail.com \
    --to=ma.uecker@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=alx@kernel.org \
    --cc=andrew@digital-domain.net \
    --cc=chris.bazley.wg14@gmail.com \
    --cc=cl@linux.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=glider@google.com \
    --cc=harry.yoo@oracle.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=kees@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mhocko@suse.com \
    --cc=pinskia@gmail.com \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=sam@gentoo.org \
    --cc=torvalds@linux-foundation.org \
    --cc=vbabka@suse.cz \
    --cc=viro@zeniv.linux.org.uk \
    --cc=~hallyn/shadow@lists.sr.ht \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).