All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lee Jones <lee@kernel.org>
To: David Laight <David.Laight@aculab.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-hardening@vger.kernel.org"
	<linux-hardening@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Petr Mladek <pmladek@suse.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Crutcher Dunnavant <crutcher+kernel@datastacks.com>,
	Juergen Quade <quade@hsnr.de>
Subject: Re: [PATCH 1/1] lib/vsprintf: Implement ssprintf() to catch truncated strings
Date: Mon, 29 Jan 2024 09:52:37 +0000	[thread overview]
Message-ID: <20240129095237.GC1708181@google.com> (raw)
In-Reply-To: <7054dcbfb7214665afedaea93ce4dbad@AcuMS.aculab.com>

On Mon, 29 Jan 2024, David Laight wrote:

> ...
> > > I'm sure that the safest return for 'truncated' is the buffer length.
> > > The a series of statements like:
> > > 	buf += xxx(buf, buf_end - buf, .....);
> > > can all be called with a single overflow check at the end.
> > >
> > > Forget the check, and the length just contains a trailing '\0'
> > > which might cause confusion but isn't going to immediately
> > > break the world.
> > 
> > snprintf() does this and has been proven to cause buffer-overflows.
> > There have been multiple articles authored describing why using
> > snprintf() is not generally a good idea for the masses including the 2
> > linked in the commit message:
> 
> snprintf() returns the number of bytes that would have been output [1].
> I'm not suggesting that, or not terminating the buffer.
> Just returning the length including the '\0' (unless length was zero).
> This still lets the code check for overflow but isn't going to
> generate a pointer outside the buffer if used to update a pointer.

I see.  Well I'm not married to my solution.  However, I am convinced
that the 2 solutions currently offered can be improved upon.  If you or
anyone else has a better solution, I'd be more than happy to implement
and switch to it.

Let me have a think about the solution you suggest and get back to you.

> [1] I'm pretty certain this is because the original libc version
> of sprintf() allocated a FILE structure on stack (fully buffered)
> and called fprintf().
> snprintf() would have been done the same way but with something
> to stop the buffer being flushed.

Interesting.  Thanks for the background.

-- 
Lee Jones [李琼斯]

  reply	other threads:[~2024-01-29  9:52 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-25  8:39 [PATCH 1/1] lib/vsprintf: Implement ssprintf() to catch truncated strings Lee Jones
2024-01-25  9:04 ` Rasmus Villemoes
2024-01-25 10:36   ` Lee Jones
2024-01-27 14:32     ` David Laight
2024-01-29  9:24       ` Lee Jones
2024-01-29  9:39         ` David Laight
2024-01-29  9:52           ` Lee Jones [this message]
2024-01-30 15:07             ` Lee Jones
2024-01-30 15:18               ` Rasmus Villemoes
2024-01-30 15:53                 ` Lee Jones
2024-02-08 16:24                   ` Petr Mladek
2024-02-08 17:05                     ` Lee Jones
2024-01-30 21:55                 ` Kees Cook
2024-01-31  8:36                   ` Lee Jones
  -- strict thread matches above, loose matches on Subject: below --
2024-01-29  9:27 Lee Jones
2024-01-29  9:31 ` Lee Jones

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=20240129095237.GC1708181@google.com \
    --to=lee@kernel.org \
    --cc=David.Laight@aculab.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=crutcher+kernel@datastacks.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=pmladek@suse.com \
    --cc=quade@hsnr.de \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.