linux-hardening - RE: [PATCH v2] checkpatch: add check for snprintf to scnprintf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dd2b4ac483b54a41aff11a04b6906fd6@AcuMS.aculab.com>
Date: Thu, 22 Feb 2024 22:30:46 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Justin Stitt' <justinstitt@...gle.com>, Andy Whitcroft
	<apw@...onical.com>, Joe Perches <joe@...ches.com>, Dwaipayan Ray
	<dwaipayanray1@...il.com>, Lukas Bulwahn <lukas.bulwahn@...il.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Lee Jones
	<lee@...nel.org>, "linux-hardening@...r.kernel.org"
	<linux-hardening@...r.kernel.org>, Kees Cook <keescook@...omium.org>, "Finn
 Thain" <fthain@...ux-m68k.org>
Subject: RE: [PATCH v2] checkpatch: add check for snprintf to scnprintf

From: Justin Stitt
> Sent: 21 February 2024 22:12
> 
> I am going to quote Lee Jones who has been doing some snprintf ->
> scnprintf refactorings:
> 
> "There is a general misunderstanding amongst engineers that
> {v}snprintf() returns the length of the data *actually* encoded into the
> destination array.  However, as per the C99 standard {v}snprintf()
> really returns the length of the data that *would have been* written if
> there were enough space for it.  This misunderstanding has led to
> buffer-overruns in the past.  It's generally considered safer to use the
> {v}scnprintf() variants in their place (or even sprintf() in simple
> cases).  So let's do that."

While generally true, there are places that really do want to
detect (and error) overflow.
That isn't possible with scnprintf().

I'm not sure what the solution is though.
Having a function that returns a negative value on overflow is also
likely to get misused.
seq_printf() (or whatever it is called) may let you check,
but it is hardly a cheap wrapper and a bit of a PITA to use.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)