linux-ext4 - Re: [PATCH v2] ext4: replace strcpy() with '.' assignment

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] [day] [month] [year] [list]

Message-ID: <20250523220719.GC332467@mit.edu>
Date: Fri, 23 May 2025 18:07:19 -0400
From: "Theodore Ts'o" <tytso@....edu>
To: Kees Cook <kees@...nel.org>
Cc: David Laight <david.laight.linux@...il.com>,
        Ethan Carter Edwards <ethan@...ancedwards.com>,
        Andreas Dilger <adilger.kernel@...ger.ca>, linux-ext4@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2] ext4: replace strcpy() with '.' assignment

On Fri, May 23, 2025 at 10:14:04AM -0700, Kees Cook wrote:
> 
> 
> On May 23, 2025 7:24:49 AM PDT, Theodore Ts'o <tytso@....edu> wrote:
> >On Fri, May 23, 2025 at 01:31:00PM +0100, David Laight wrote:
> >> 
> >> The compiler (or headers files) can also allow strcpy() of constant
> >> length strings into arrays (known size). Erroring requests that are too long.
> >> The strcpy() is then converted to a memcpy() which can then be optimised
> >> into writes of constants.
> >> 
> >> So using strcpy() under those conditions 'isn't all bad' and can generate
> >> better (and less bug prone) code than trying to hand-optimise it.
> >> 
> >> So even through strcpy() is usually a bad idea, there is not need to
> >> remove the calls that the compiler can validate as safe.
> >
> >I assume that what the hardening folks want to do is to assert that
> >strcpy is always evil(tm) so they can detect potential security bugs
> >by doing "git grep strcpy".
> 
> FWIW, what I'd like is a lack of ambiguity for both humans and
> compilers. "Get rid of strcpy" is the Big Hammer solution for
> strcpy. The more precise version is "disallow strcpy of a src or dst
> where either lack a compile-time buffer size".

Well, technically speaking struct ext4_dir_entry.name has a fixed
compile-time buffer size:

struct ext4_dir_entry {
	__le32	inode;			/* Inode number */
	__le16	rec_len;		/* Directory entry length */
	__le16	name_len;		/* Name length */
	char	name[EXT4_NAME_LEN];	/* File name */
};

And what we're copying into name here is also fixed.  It's either "."
or "..".   As far as optimization is concerned, whether

   de->name[0] = de->name[1] = '.';

could be better optimized by the compiler than:

   strcpy(de->name, "..");
or
   memcpy(de->name, "..", 2);
(which is all that is required)

Meh.  Probably the compiler could optimized it into a 2-byte word
store, but it's not like mkdir is a hot path.  :-)

It's probably easier to patch the code path and as opposed to having
the conversation about how "no, really, it's safe, and I can prove
it."  If this was a performance hot path, I might care more, but it
isn't, so I don't.

           	   	    	       - Ted