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

Ethan Carter Edwards posted 1 patch 7 months ago
There is a newer version of this series
fs/ext4/inline.c | 4 ++--
fs/ext4/namei.c  | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
[PATCH v2] ext4: replace strcpy() with '.' assignment
Posted by Ethan Carter Edwards 7 months ago
strcpy() is deprecated; assignment can be used instead which
theoretically/potentially increases speed as a function call is removed.

Straight assignment works because the strings are not null-terminated
which means they don't strictly require a str(s)cpy call.

No functional changes intended.

Link: https://github.com/KSPP/linux/issues/88
Suggested-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
---
There's an ongoing effort to remove calls to strcpy throughout the
kernel.

Link: https://github.com/KSPP/linux/issues/88
---
Changes in v2:
- completely remove the call to strcpy and replace it with assignment
  off of Theo's suggestion. Thanks.
- Link to v1: https://lore.kernel.org/r/20250518-ext4-strcpy-v1-1-6c8a82ff078f@ethancedwards.com
---
 fs/ext4/inline.c | 4 ++--
 fs/ext4/namei.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 2c9b762925c72f2ff5a402b02500370bc1eb0eb1..f3bc8b3904a8a9b55162f002b5bd63a527b290a5 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1314,7 +1314,7 @@ int ext4_inlinedir_to_tree(struct file *dir_file,
 		if (pos == 0) {
 			fake.inode = cpu_to_le32(inode->i_ino);
 			fake.name_len = 1;
-			strcpy(fake.name, ".");
+			fake.name[0] = ".";
 			fake.rec_len = ext4_rec_len_to_disk(
 					  ext4_dir_rec_len(fake.name_len, NULL),
 					  inline_size);
@@ -1324,7 +1324,7 @@ int ext4_inlinedir_to_tree(struct file *dir_file,
 		} else if (pos == EXT4_INLINE_DOTDOT_OFFSET) {
 			fake.inode = cpu_to_le32(parent_ino);
 			fake.name_len = 2;
-			strcpy(fake.name, "..");
+			fake.name[0] = fake.name[1] = ".";
 			fake.rec_len = ext4_rec_len_to_disk(
 					  ext4_dir_rec_len(fake.name_len, NULL),
 					  inline_size);
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index e9712e64ec8f04586f5ebcd332431e6af92e4f36..c7d7c46a0b18ae109d30358c157812ac2ded200e 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2926,7 +2926,7 @@ struct ext4_dir_entry_2 *ext4_init_dot_dotdot(struct inode *inode,
 	de->name_len = 1;
 	de->rec_len = ext4_rec_len_to_disk(ext4_dir_rec_len(de->name_len, NULL),
 					   blocksize);
-	strcpy(de->name, ".");
+	de->name[0] = '.';
 	ext4_set_de_type(inode->i_sb, de, S_IFDIR);
 
 	de = ext4_next_entry(de, blocksize);
@@ -2940,7 +2940,7 @@ struct ext4_dir_entry_2 *ext4_init_dot_dotdot(struct inode *inode,
 		de->rec_len = ext4_rec_len_to_disk(
 					ext4_dir_rec_len(de->name_len, NULL),
 					blocksize);
-	strcpy(de->name, "..");
+	de->name[0] = de->name[1] = '.';
 	ext4_set_de_type(inode->i_sb, de, S_IFDIR);
 
 	return ext4_next_entry(de, blocksize);

---
base-commit: 5723cc3450bccf7f98f227b9723b5c9f6b3af1c5
change-id: 20250518-ext4-strcpy-1545c6f79b51

Best regards,
-- 
Ethan Carter Edwards <ethan@ethancedwards.com>
Re: [PATCH v2] ext4: replace strcpy() with '.' assignment
Posted by Kees Cook 7 months ago
On Sun, May 18, 2025 at 11:54:21PM -0400, Ethan Carter Edwards wrote:
> strcpy() is deprecated; assignment can be used instead which
> theoretically/potentially increases speed as a function call is removed.
> 
> Straight assignment works because the strings are not null-terminated
> which means they don't strictly require a str(s)cpy call.
> 
> No functional changes intended.
> 
> Link: https://github.com/KSPP/linux/issues/88
> Suggested-by: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
> ---
> There's an ongoing effort to remove calls to strcpy throughout the
> kernel.
> 
> Link: https://github.com/KSPP/linux/issues/88
> ---
> Changes in v2:
> - completely remove the call to strcpy and replace it with assignment
>   off of Theo's suggestion. Thanks.
> - Link to v1: https://lore.kernel.org/r/20250518-ext4-strcpy-v1-1-6c8a82ff078f@ethancedwards.com
> ---
>  fs/ext4/inline.c | 4 ++--
>  fs/ext4/namei.c  | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 2c9b762925c72f2ff5a402b02500370bc1eb0eb1..f3bc8b3904a8a9b55162f002b5bd63a527b290a5 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -1314,7 +1314,7 @@ int ext4_inlinedir_to_tree(struct file *dir_file,
>  		if (pos == 0) {
>  			fake.inode = cpu_to_le32(inode->i_ino);
>  			fake.name_len = 1;
> -			strcpy(fake.name, ".");
> +			fake.name[0] = ".";

This means the trailing NUL byte isn't being copied any more? That seems
like a big change, even if name_len is being used for length tracking.

-- 
Kees Cook
Re: [PATCH v2] ext4: replace strcpy() with '.' assignment
Posted by Theodore Ts'o 7 months ago
On Mon, May 19, 2025 at 06:52:13AM -0700, Kees Cook wrote:
> > --- a/fs/ext4/inline.c
> > +++ b/fs/ext4/inline.c
> > @@ -1314,7 +1314,7 @@ int ext4_inlinedir_to_tree(struct file *dir_file,
> >  		if (pos == 0) {
> >  			fake.inode = cpu_to_le32(inode->i_ino);
> >  			fake.name_len = 1;
> > -			strcpy(fake.name, ".");
> > +			fake.name[0] = ".";
> 
> This means the trailing NUL byte isn't being copied any more? That seems
> like a big change, even if name_len is being used for length tracking.

Yeah, and so that's something that needs to be tested (and not just
build tested to catch the obvious FTBFS bug).  However, note how we
handle normal filenames, as opposed to "." and "..".  From
ext4_insert_dentry():

	de->inode = cpu_to_le32(inode->i_ino);
	ext4_set_de_type(inode->i_sb, de, inode->i_mode);
	de->name_len = fname_len(fname);
	memcpy(de->name, fname_name(fname), fname_len(fname));

Or were you commenting on the "no functional changes intended" line in
the commit description?  I agree that this is probably no longer
accurate.  :-)

					- Ted
Re: [PATCH v2] ext4: replace strcpy() with '.' assignment
Posted by David Laight 7 months ago
On Mon, 19 May 2025 10:59:30 -0400
"Theodore Ts'o" <tytso@mit.edu> wrote:

> On Mon, May 19, 2025 at 06:52:13AM -0700, Kees Cook wrote:
> > > --- a/fs/ext4/inline.c
> > > +++ b/fs/ext4/inline.c
> > > @@ -1314,7 +1314,7 @@ int ext4_inlinedir_to_tree(struct file *dir_file,
> > >  		if (pos == 0) {
> > >  			fake.inode = cpu_to_le32(inode->i_ino);
> > >  			fake.name_len = 1;
> > > -			strcpy(fake.name, ".");
> > > +			fake.name[0] = ".";  
> > 
> > This means the trailing NUL byte isn't being copied any more? That seems
> > like a big change, even if name_len is being used for length tracking.  
> 
> Yeah, and so that's something that needs to be tested (and not just
> build tested to catch the obvious FTBFS bug).

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.

	David   

> However, note how we
> handle normal filenames, as opposed to "." and "..".  From
> ext4_insert_dentry():
> 
> 	de->inode = cpu_to_le32(inode->i_ino);
> 	ext4_set_de_type(inode->i_sb, de, inode->i_mode);
> 	de->name_len = fname_len(fname);
> 	memcpy(de->name, fname_name(fname), fname_len(fname));
> 
> Or were you commenting on the "no functional changes intended" line in
> the commit description?  I agree that this is probably no longer
> accurate.  :-)
> 
> 					- Ted
>
Re: [PATCH v2] ext4: replace strcpy() with '.' assignment
Posted by Theodore Ts'o 6 months, 4 weeks ago
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".

						- Ted
Re: [PATCH v2] ext4: replace strcpy() with '.' assignment
Posted by Kees Cook 6 months, 4 weeks ago

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".

-Kees


-- 
Kees Cook
Re: [PATCH v2] ext4: replace strcpy() with '.' assignment
Posted by Theodore Ts'o 6 months, 4 weeks ago
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
Re: [PATCH v2] ext4: replace strcpy() with '.' assignment
Posted by kernel test robot 7 months ago
Hi Ethan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 5723cc3450bccf7f98f227b9723b5c9f6b3af1c5]

url:    https://github.com/intel-lab-lkp/linux/commits/Ethan-Carter-Edwards/ext4-replace-strcpy-with-assignment/20250519-115601
base:   5723cc3450bccf7f98f227b9723b5c9f6b3af1c5
patch link:    https://lore.kernel.org/r/20250518-ext4-strcpy-v2-1-80d316325046%40ethancedwards.com
patch subject: [PATCH v2] ext4: replace strcpy() with '.' assignment
config: csky-randconfig-001-20250519 (https://download.01.org/0day-ci/archive/20250519/202505191321.ZwdQZhut-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250519/202505191321.ZwdQZhut-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505191321.ZwdQZhut-lkp@intel.com/

All warnings (new ones prefixed by >>):

   fs/ext4/inline.c: In function 'ext4_inlinedir_to_tree':
>> fs/ext4/inline.c:1317:17: warning: assignment to 'char' from 'char *' makes integer from pointer without a cast [-Wint-conversion]
    1317 |    fake.name[0] = ".";
         |                 ^
   fs/ext4/inline.c:1327:32: warning: assignment to 'char' from 'char *' makes integer from pointer without a cast [-Wint-conversion]
    1327 |    fake.name[0] = fake.name[1] = ".";
         |                                ^


vim +1317 fs/ext4/inline.c

  1259	
  1260	/*
  1261	 * This function fills a red-black tree with information from an
  1262	 * inlined dir.  It returns the number directory entries loaded
  1263	 * into the tree.  If there is an error it is returned in err.
  1264	 */
  1265	int ext4_inlinedir_to_tree(struct file *dir_file,
  1266				   struct inode *dir, ext4_lblk_t block,
  1267				   struct dx_hash_info *hinfo,
  1268				   __u32 start_hash, __u32 start_minor_hash,
  1269				   int *has_inline_data)
  1270	{
  1271		int err = 0, count = 0;
  1272		unsigned int parent_ino;
  1273		int pos;
  1274		struct ext4_dir_entry_2 *de;
  1275		struct inode *inode = file_inode(dir_file);
  1276		int ret, inline_size = 0;
  1277		struct ext4_iloc iloc;
  1278		void *dir_buf = NULL;
  1279		struct ext4_dir_entry_2 fake;
  1280		struct fscrypt_str tmp_str;
  1281	
  1282		ret = ext4_get_inode_loc(inode, &iloc);
  1283		if (ret)
  1284			return ret;
  1285	
  1286		down_read(&EXT4_I(inode)->xattr_sem);
  1287		if (!ext4_has_inline_data(inode)) {
  1288			up_read(&EXT4_I(inode)->xattr_sem);
  1289			*has_inline_data = 0;
  1290			goto out;
  1291		}
  1292	
  1293		inline_size = ext4_get_inline_size(inode);
  1294		dir_buf = kmalloc(inline_size, GFP_NOFS);
  1295		if (!dir_buf) {
  1296			ret = -ENOMEM;
  1297			up_read(&EXT4_I(inode)->xattr_sem);
  1298			goto out;
  1299		}
  1300	
  1301		ret = ext4_read_inline_data(inode, dir_buf, inline_size, &iloc);
  1302		up_read(&EXT4_I(inode)->xattr_sem);
  1303		if (ret < 0)
  1304			goto out;
  1305	
  1306		pos = 0;
  1307		parent_ino = le32_to_cpu(((struct ext4_dir_entry_2 *)dir_buf)->inode);
  1308		while (pos < inline_size) {
  1309			/*
  1310			 * As inlined dir doesn't store any information about '.' and
  1311			 * only the inode number of '..' is stored, we have to handle
  1312			 * them differently.
  1313			 */
  1314			if (pos == 0) {
  1315				fake.inode = cpu_to_le32(inode->i_ino);
  1316				fake.name_len = 1;
> 1317				fake.name[0] = ".";
  1318				fake.rec_len = ext4_rec_len_to_disk(
  1319						  ext4_dir_rec_len(fake.name_len, NULL),
  1320						  inline_size);
  1321				ext4_set_de_type(inode->i_sb, &fake, S_IFDIR);
  1322				de = &fake;
  1323				pos = EXT4_INLINE_DOTDOT_OFFSET;
  1324			} else if (pos == EXT4_INLINE_DOTDOT_OFFSET) {
  1325				fake.inode = cpu_to_le32(parent_ino);
  1326				fake.name_len = 2;
  1327				fake.name[0] = fake.name[1] = ".";
  1328				fake.rec_len = ext4_rec_len_to_disk(
  1329						  ext4_dir_rec_len(fake.name_len, NULL),
  1330						  inline_size);
  1331				ext4_set_de_type(inode->i_sb, &fake, S_IFDIR);
  1332				de = &fake;
  1333				pos = EXT4_INLINE_DOTDOT_SIZE;
  1334			} else {
  1335				de = (struct ext4_dir_entry_2 *)(dir_buf + pos);
  1336				pos += ext4_rec_len_from_disk(de->rec_len, inline_size);
  1337				if (ext4_check_dir_entry(inode, dir_file, de,
  1338						 iloc.bh, dir_buf,
  1339						 inline_size, pos)) {
  1340					ret = count;
  1341					goto out;
  1342				}
  1343			}
  1344	
  1345			if (ext4_hash_in_dirent(dir)) {
  1346				hinfo->hash = EXT4_DIRENT_HASH(de);
  1347				hinfo->minor_hash = EXT4_DIRENT_MINOR_HASH(de);
  1348			} else {
  1349				err = ext4fs_dirhash(dir, de->name, de->name_len, hinfo);
  1350				if (err) {
  1351					ret = err;
  1352					goto out;
  1353				}
  1354			}
  1355			if ((hinfo->hash < start_hash) ||
  1356			    ((hinfo->hash == start_hash) &&
  1357			     (hinfo->minor_hash < start_minor_hash)))
  1358				continue;
  1359			if (de->inode == 0)
  1360				continue;
  1361			tmp_str.name = de->name;
  1362			tmp_str.len = de->name_len;
  1363			err = ext4_htree_store_dirent(dir_file, hinfo->hash,
  1364						      hinfo->minor_hash, de, &tmp_str);
  1365			if (err) {
  1366				ret = err;
  1367				goto out;
  1368			}
  1369			count++;
  1370		}
  1371		ret = count;
  1372	out:
  1373		kfree(dir_buf);
  1374		brelse(iloc.bh);
  1375		return ret;
  1376	}
  1377	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v2] ext4: replace strcpy() with '.' assignment
Posted by kernel test robot 7 months ago
Hi Ethan,

kernel test robot noticed the following build errors:

[auto build test ERROR on 5723cc3450bccf7f98f227b9723b5c9f6b3af1c5]

url:    https://github.com/intel-lab-lkp/linux/commits/Ethan-Carter-Edwards/ext4-replace-strcpy-with-assignment/20250519-115601
base:   5723cc3450bccf7f98f227b9723b5c9f6b3af1c5
patch link:    https://lore.kernel.org/r/20250518-ext4-strcpy-v2-1-80d316325046%40ethancedwards.com
patch subject: [PATCH v2] ext4: replace strcpy() with '.' assignment
config: csky-randconfig-002-20250519 (https://download.01.org/0day-ci/archive/20250519/202505191316.JJMnPobO-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250519/202505191316.JJMnPobO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505191316.JJMnPobO-lkp@intel.com/

All errors (new ones prefixed by >>):

   fs/ext4/inline.c: In function 'ext4_inlinedir_to_tree':
>> fs/ext4/inline.c:1317:38: error: assignment to 'char' from 'char *' makes integer from pointer without a cast [-Wint-conversion]
    1317 |                         fake.name[0] = ".";
         |                                      ^
   fs/ext4/inline.c:1327:53: error: assignment to 'char' from 'char *' makes integer from pointer without a cast [-Wint-conversion]
    1327 |                         fake.name[0] = fake.name[1] = ".";
         |                                                     ^


vim +1317 fs/ext4/inline.c

  1259	
  1260	/*
  1261	 * This function fills a red-black tree with information from an
  1262	 * inlined dir.  It returns the number directory entries loaded
  1263	 * into the tree.  If there is an error it is returned in err.
  1264	 */
  1265	int ext4_inlinedir_to_tree(struct file *dir_file,
  1266				   struct inode *dir, ext4_lblk_t block,
  1267				   struct dx_hash_info *hinfo,
  1268				   __u32 start_hash, __u32 start_minor_hash,
  1269				   int *has_inline_data)
  1270	{
  1271		int err = 0, count = 0;
  1272		unsigned int parent_ino;
  1273		int pos;
  1274		struct ext4_dir_entry_2 *de;
  1275		struct inode *inode = file_inode(dir_file);
  1276		int ret, inline_size = 0;
  1277		struct ext4_iloc iloc;
  1278		void *dir_buf = NULL;
  1279		struct ext4_dir_entry_2 fake;
  1280		struct fscrypt_str tmp_str;
  1281	
  1282		ret = ext4_get_inode_loc(inode, &iloc);
  1283		if (ret)
  1284			return ret;
  1285	
  1286		down_read(&EXT4_I(inode)->xattr_sem);
  1287		if (!ext4_has_inline_data(inode)) {
  1288			up_read(&EXT4_I(inode)->xattr_sem);
  1289			*has_inline_data = 0;
  1290			goto out;
  1291		}
  1292	
  1293		inline_size = ext4_get_inline_size(inode);
  1294		dir_buf = kmalloc(inline_size, GFP_NOFS);
  1295		if (!dir_buf) {
  1296			ret = -ENOMEM;
  1297			up_read(&EXT4_I(inode)->xattr_sem);
  1298			goto out;
  1299		}
  1300	
  1301		ret = ext4_read_inline_data(inode, dir_buf, inline_size, &iloc);
  1302		up_read(&EXT4_I(inode)->xattr_sem);
  1303		if (ret < 0)
  1304			goto out;
  1305	
  1306		pos = 0;
  1307		parent_ino = le32_to_cpu(((struct ext4_dir_entry_2 *)dir_buf)->inode);
  1308		while (pos < inline_size) {
  1309			/*
  1310			 * As inlined dir doesn't store any information about '.' and
  1311			 * only the inode number of '..' is stored, we have to handle
  1312			 * them differently.
  1313			 */
  1314			if (pos == 0) {
  1315				fake.inode = cpu_to_le32(inode->i_ino);
  1316				fake.name_len = 1;
> 1317				fake.name[0] = ".";
  1318				fake.rec_len = ext4_rec_len_to_disk(
  1319						  ext4_dir_rec_len(fake.name_len, NULL),
  1320						  inline_size);
  1321				ext4_set_de_type(inode->i_sb, &fake, S_IFDIR);
  1322				de = &fake;
  1323				pos = EXT4_INLINE_DOTDOT_OFFSET;
  1324			} else if (pos == EXT4_INLINE_DOTDOT_OFFSET) {
  1325				fake.inode = cpu_to_le32(parent_ino);
  1326				fake.name_len = 2;
  1327				fake.name[0] = fake.name[1] = ".";
  1328				fake.rec_len = ext4_rec_len_to_disk(
  1329						  ext4_dir_rec_len(fake.name_len, NULL),
  1330						  inline_size);
  1331				ext4_set_de_type(inode->i_sb, &fake, S_IFDIR);
  1332				de = &fake;
  1333				pos = EXT4_INLINE_DOTDOT_SIZE;
  1334			} else {
  1335				de = (struct ext4_dir_entry_2 *)(dir_buf + pos);
  1336				pos += ext4_rec_len_from_disk(de->rec_len, inline_size);
  1337				if (ext4_check_dir_entry(inode, dir_file, de,
  1338						 iloc.bh, dir_buf,
  1339						 inline_size, pos)) {
  1340					ret = count;
  1341					goto out;
  1342				}
  1343			}
  1344	
  1345			if (ext4_hash_in_dirent(dir)) {
  1346				hinfo->hash = EXT4_DIRENT_HASH(de);
  1347				hinfo->minor_hash = EXT4_DIRENT_MINOR_HASH(de);
  1348			} else {
  1349				err = ext4fs_dirhash(dir, de->name, de->name_len, hinfo);
  1350				if (err) {
  1351					ret = err;
  1352					goto out;
  1353				}
  1354			}
  1355			if ((hinfo->hash < start_hash) ||
  1356			    ((hinfo->hash == start_hash) &&
  1357			     (hinfo->minor_hash < start_minor_hash)))
  1358				continue;
  1359			if (de->inode == 0)
  1360				continue;
  1361			tmp_str.name = de->name;
  1362			tmp_str.len = de->name_len;
  1363			err = ext4_htree_store_dirent(dir_file, hinfo->hash,
  1364						      hinfo->minor_hash, de, &tmp_str);
  1365			if (err) {
  1366				ret = err;
  1367				goto out;
  1368			}
  1369			count++;
  1370		}
  1371		ret = count;
  1372	out:
  1373		kfree(dir_buf);
  1374		brelse(iloc.bh);
  1375		return ret;
  1376	}
  1377	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v2] ext4: replace strcpy() with '.' assignment
Posted by Theodore Ts'o 7 months ago
On Mon, May 19, 2025 at 01:58:02PM +0800, kernel test robot wrote:
> Hi Ethan,
> 
> kernel test robot noticed the following build errors:

Hi Ethan,

I would really appreciate it if you would at least do a build test
before sending out a patch.  In addition, it would also be helpful if
you ran a smoke test, using "kvm-xfstests smoke".  The instructions
for how to use kvm-xfstests can be found here[1], and it was designed
to be as easy as possible for people who are sending "drive-by
patches".

[1] https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-quickstart.md

Running the smoketest only takes about 20 minutes; although if you
want to run more sophistcated testing there is also "kvm-xfstests -c
ext4/4k -g quick", or "kvm-xfstests -c ext4/4k -g auto", or if you
have a 24 hours to kill, there's always "kvm-xfststs full".  (Although
these days I generally use gce-xfstests[2] since this can shard the test
runs across multiple VM's, so it only takes 2.5 hours of wall clock
time.)

[2] https://thunk.org/gce-xfstests

Cheers,

					- TEd