public inbox for linux-trace-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Josh Law <objecting@objecting.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v4] lib/bootconfig: guard xbc_node_compose_key_after() buffer size
Date: Wed, 18 Mar 2026 09:02:43 +0900	[thread overview]
Message-ID: <20260318090243.7c437f2c5e07a1ce00375102@kernel.org> (raw)
In-Reply-To: <20260317191626.5b6172a9@robin>

On Tue, 17 Mar 2026 19:16:26 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 18 Mar 2026 08:03:51 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > On Tue, 17 Mar 2026 20:44:03 +0000
> > Josh Law <objecting@objecting.org> wrote:
> > 
> > > xbc_node_compose_key_after() passes a size_t buffer length to
> > > snprintf(), but snprintf() returns int. Guard against size values above
> > > INT_MAX before the loop so the existing truncation check can continue to
> > > compare ret against (int)size safely.
> > > 
> > > Add a small WARN_ON_ONCE shim for the tools/bootconfig userspace build
> > > so the same source continues to build there.  
> > 
> > NACK.
> > 
> > Don't do such over engineering effort.
> 
> Hi Masami,
> 
> This was somewhat my idea. Why do you think it's over engineering?
> 
> This is your code, so you have final say. I'm not going to push it. I'm
> just curious to your thoughts.

I sent a mail why I thought this is over engineering. I think this
comes from vsnprintf() interface design. If all user of that needs
to do this, that is not fair. It should be checked in vsnprintf()
and caller should just check the returned error.

> 
> It is interesting that snprintf() takes a size_t size, and the iterator
> inside is also size_t, but then it returns the value as an int.

Yes, that is checked in vsnprintf(), not its caller.
I think linux kernel should ensure the the return value is smaller
than INT_MAX, and return -EOVERFLOW if not.

Thank you,

> 
> That itself just looks wrong (and has nothing to do with your code).
> 
> -- Steve


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

  reply	other threads:[~2026-03-18  0:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-17 20:44 [PATCH v4] lib/bootconfig: guard xbc_node_compose_key_after() buffer size Josh Law
2026-03-17 23:03 ` Masami Hiramatsu
2026-03-17 23:16   ` Steven Rostedt
2026-03-18  0:02     ` Masami Hiramatsu [this message]
2026-03-18  0:43       ` Steven Rostedt
2026-03-18  3:07         ` Masami Hiramatsu
2026-03-18 13:45       ` Steven Rostedt

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=20260318090243.7c437f2c5e07a1ce00375102@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=objecting@objecting.org \
    --cc=rostedt@goodmis.org \
    /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