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

Theodore Ts'o
2025-05-23T22:07:41Z

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@mit.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