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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx> 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




[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux