* [PATCH 1/3] ext4: replace strcmp with direct comparison for '.' and '..'
@ 2025-07-12 18:12 Theodore Ts'o
2025-07-12 18:12 ` [PATCH 2/3] ext4: use memcpy() instead of strcpy() Theodore Ts'o
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Theodore Ts'o @ 2025-07-12 18:12 UTC (permalink / raw)
To: Ext4 Developers List; +Cc: linux-hardening, ethan, Theodore Ts'o
In a discussion over a proposed patch, "ext4: replace strcpy() with
'.' assignment"[1], I had asserted that directory entries in ext4 were
not NUL terminated, and hence it was safe to replace strcpy() with a
direct assignment. As it turns out, this was incorrect. It's true
for all all directory entries *except* for '.' and '..' where the
kernel was using strcmp() and where e2fsck actually checks and offers
to fix things if '.' and '..' are not NUL terminated.
[1] https://lore.kernel.org/r/202505191316.JJMnPobO-lkp@intel.com
We can't change this without breaking old kernel versions, but in the
spirit of "be liberal in what you receive", use direct comparison of
de->name_len and de->name[0,1] instead of strcmp(). This has the side
benefit of reducing the compiled text size by 96 bytes on x86_64.
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
fs/ext4/namei.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index a178ac229489..b82f5841c65a 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -3082,7 +3082,8 @@ bool ext4_empty_dir(struct inode *inode)
de = (struct ext4_dir_entry_2 *) bh->b_data;
if (ext4_check_dir_entry(inode, NULL, de, bh, bh->b_data, bh->b_size,
0) ||
- le32_to_cpu(de->inode) != inode->i_ino || strcmp(".", de->name)) {
+ le32_to_cpu(de->inode) != inode->i_ino || de->name_len != 1 ||
+ de->name[0] != '.') {
ext4_warning_inode(inode, "directory missing '.'");
brelse(bh);
return false;
@@ -3091,7 +3092,8 @@ bool ext4_empty_dir(struct inode *inode)
de = ext4_next_entry(de, sb->s_blocksize);
if (ext4_check_dir_entry(inode, NULL, de, bh, bh->b_data, bh->b_size,
offset) ||
- le32_to_cpu(de->inode) == 0 || strcmp("..", de->name)) {
+ le32_to_cpu(de->inode) == 0 || de->name_len != 2 ||
+ de->name[0] != '.' || de->name[1] != '.') {
ext4_warning_inode(inode, "directory missing '..'");
brelse(bh);
return false;
@@ -3532,7 +3534,7 @@ static struct buffer_head *ext4_get_first_dir_block(handle_t *handle,
if (ext4_check_dir_entry(inode, NULL, de, bh, bh->b_data,
bh->b_size, 0) ||
le32_to_cpu(de->inode) != inode->i_ino ||
- strcmp(".", de->name)) {
+ de->name_len != 1 || de->name[0] != '.') {
EXT4_ERROR_INODE(inode, "directory missing '.'");
brelse(bh);
*retval = -EFSCORRUPTED;
@@ -3543,7 +3545,8 @@ static struct buffer_head *ext4_get_first_dir_block(handle_t *handle,
de = ext4_next_entry(de, inode->i_sb->s_blocksize);
if (ext4_check_dir_entry(inode, NULL, de, bh, bh->b_data,
bh->b_size, offset) ||
- le32_to_cpu(de->inode) == 0 || strcmp("..", de->name)) {
+ le32_to_cpu(de->inode) == 0 || de->name_len != 2 ||
+ de->name[0] != '.' || de->name[1] != '.') {
EXT4_ERROR_INODE(inode, "directory missing '..'");
brelse(bh);
*retval = -EFSCORRUPTED;
--
2.47.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] ext4: use memcpy() instead of strcpy()
2025-07-12 18:12 [PATCH 1/3] ext4: replace strcmp with direct comparison for '.' and '..' Theodore Ts'o
@ 2025-07-12 18:12 ` Theodore Ts'o
2025-07-30 15:02 ` Andy Shevchenko
2025-07-12 18:12 ` [PATCH 3/3] ext4: refactor the inline directory conversion and new directory codepaths Theodore Ts'o
2025-07-19 21:45 ` [PATCH 1/3] ext4: replace strcmp with direct comparison for '.' and '..' Theodore Ts'o
2 siblings, 1 reply; 8+ messages in thread
From: Theodore Ts'o @ 2025-07-12 18:12 UTC (permalink / raw)
To: Ext4 Developers List; +Cc: linux-hardening, ethan, Theodore Ts'o
The strcpy() function is considered dangerous and eeeevil by people
who are using sophisticated code analysis tools such as "grep". This
is true even when a quick inspection would show that the source is a
constant string ("." or "..") and the destination is a fixed array
which is guaranteed to have enough space. Make the "grep" code
analysis tool happy by using memcpy() isstead of strcpy(). :-)
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
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 a1bbcdf40824..eeee007251e0 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1315,7 +1315,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, ".");
+ memcpy(fake.name, ".", 2);
fake.rec_len = ext4_rec_len_to_disk(
ext4_dir_rec_len(fake.name_len, NULL),
inline_size);
@@ -1325,7 +1325,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, "..");
+ memcpy(fake.name, "..", 3);
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 b82f5841c65a..9913a94b6a6d 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2924,7 +2924,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, ".");
+ memcpy(de->name, ".", 2);
ext4_set_de_type(inode->i_sb, de, S_IFDIR);
de = ext4_next_entry(de, blocksize);
@@ -2938,7 +2938,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, "..");
+ memcpy(de->name, "..", 3);
ext4_set_de_type(inode->i_sb, de, S_IFDIR);
return ext4_next_entry(de, blocksize);
--
2.47.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] ext4: refactor the inline directory conversion and new directory codepaths
2025-07-12 18:12 [PATCH 1/3] ext4: replace strcmp with direct comparison for '.' and '..' Theodore Ts'o
2025-07-12 18:12 ` [PATCH 2/3] ext4: use memcpy() instead of strcpy() Theodore Ts'o
@ 2025-07-12 18:12 ` Theodore Ts'o
2025-07-12 21:12 ` kernel test robot
2025-07-19 21:45 ` [PATCH 1/3] ext4: replace strcmp with direct comparison for '.' and '..' Theodore Ts'o
2 siblings, 1 reply; 8+ messages in thread
From: Theodore Ts'o @ 2025-07-12 18:12 UTC (permalink / raw)
To: Ext4 Developers List; +Cc: linux-hardening, ethan, Theodore Ts'o
There was a lot of common code in the codepaths used to convert an
inline directory and to creaet a new directory. To address this,
rename ext4_init_dot_dotdot() to ext4_init_dirblock() and then move
common code into that function.
This reduces the lines of code count in fs/ext4/inline.c and
fs/ext4/namei.c, as well as reducing the size of their object files.
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
fs/ext4/ext4.h | 9 ++++----
fs/ext4/inline.c | 60 ++++++++++--------------------------------------
fs/ext4/namei.c | 56 ++++++++++++++++++++++++--------------------
3 files changed, 48 insertions(+), 77 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 18373de980f2..4d1d3eff2418 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3612,6 +3612,7 @@ extern loff_t ext4_llseek(struct file *file, loff_t offset, int origin);
extern int ext4_get_max_inline_size(struct inode *inode);
extern int ext4_find_inline_data_nolock(struct inode *inode);
extern int ext4_destroy_inline_data(handle_t *handle, struct inode *inode);
+extern void ext4_update_final_de(void *de_buf, int old_size, int new_size);
int ext4_readpage_inline(struct inode *inode, struct folio *folio);
extern int ext4_try_to_write_inline_data(struct address_space *mapping,
@@ -3671,10 +3672,10 @@ static inline int ext4_has_inline_data(struct inode *inode)
extern const struct inode_operations ext4_dir_inode_operations;
extern const struct inode_operations ext4_special_inode_operations;
extern struct dentry *ext4_get_parent(struct dentry *child);
-extern struct ext4_dir_entry_2 *ext4_init_dot_dotdot(struct inode *inode,
- struct ext4_dir_entry_2 *de,
- int blocksize, int csum_size,
- unsigned int parent_ino, int dotdot_real_len);
+extern int ext4_init_dirblock(handle_t *handle, struct inode *inode,
+ struct buffer_head *dir_block,
+ unsigned int parent_ino, void *inline_buf,
+ int inline_size);
extern void ext4_initialize_dirent_tail(struct buffer_head *bh,
unsigned int blocksize);
extern int ext4_handle_dirty_dirblock(handle_t *handle, struct inode *inode,
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index eeee007251e0..4ad3255e45cb 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -995,7 +995,7 @@ static void *ext4_get_inline_xattr_pos(struct inode *inode,
}
/* Set the final de to cover the whole block. */
-static void ext4_update_final_de(void *de_buf, int old_size, int new_size)
+void ext4_update_final_de(void *de_buf, int old_size, int new_size)
{
struct ext4_dir_entry_2 *de, *prev_de;
void *limit;
@@ -1059,51 +1059,6 @@ static void ext4_restore_inline_data(handle_t *handle, struct inode *inode,
ext4_set_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
}
-static int ext4_finish_convert_inline_dir(handle_t *handle,
- struct inode *inode,
- struct buffer_head *dir_block,
- void *buf,
- int inline_size)
-{
- int err, csum_size = 0, header_size = 0;
- struct ext4_dir_entry_2 *de;
- void *target = dir_block->b_data;
-
- /*
- * First create "." and ".." and then copy the dir information
- * back to the block.
- */
- de = target;
- de = ext4_init_dot_dotdot(inode, de,
- inode->i_sb->s_blocksize, csum_size,
- le32_to_cpu(((struct ext4_dir_entry_2 *)buf)->inode), 1);
- header_size = (void *)de - target;
-
- memcpy((void *)de, buf + EXT4_INLINE_DOTDOT_SIZE,
- inline_size - EXT4_INLINE_DOTDOT_SIZE);
-
- if (ext4_has_feature_metadata_csum(inode->i_sb))
- csum_size = sizeof(struct ext4_dir_entry_tail);
-
- inode->i_size = inode->i_sb->s_blocksize;
- i_size_write(inode, inode->i_sb->s_blocksize);
- EXT4_I(inode)->i_disksize = inode->i_sb->s_blocksize;
- ext4_update_final_de(dir_block->b_data,
- inline_size - EXT4_INLINE_DOTDOT_SIZE + header_size,
- inode->i_sb->s_blocksize - csum_size);
-
- if (csum_size)
- ext4_initialize_dirent_tail(dir_block,
- inode->i_sb->s_blocksize);
- set_buffer_uptodate(dir_block);
- unlock_buffer(dir_block);
- err = ext4_handle_dirty_dirblock(handle, inode, dir_block);
- if (err)
- return err;
- set_buffer_verified(dir_block);
- return ext4_mark_inode_dirty(handle, inode);
-}
-
static int ext4_convert_inline_data_nolock(handle_t *handle,
struct inode *inode,
struct ext4_iloc *iloc)
@@ -1175,8 +1130,17 @@ static int ext4_convert_inline_data_nolock(handle_t *handle,
error = ext4_handle_dirty_metadata(handle,
inode, data_bh);
} else {
- error = ext4_finish_convert_inline_dir(handle, inode, data_bh,
- buf, inline_size);
+ unlock_buffer(data_bh);
+ inode->i_size = inode->i_sb->s_blocksize;
+ i_size_write(inode, inode->i_sb->s_blocksize);
+ EXT4_I(inode)->i_disksize = inode->i_sb->s_blocksize;
+
+ error = ext4_init_dirblock(handle, inode, data_bh,
+ le32_to_cpu(((struct ext4_dir_entry_2 *)buf)->inode),
+ buf + EXT4_INLINE_DOTDOT_SIZE,
+ inline_size - EXT4_INLINE_DOTDOT_SIZE);
+ if (!error)
+ error = ext4_mark_inode_dirty(handle, inode);
}
out_restore:
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 9913a94b6a6d..d83f91b62317 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2915,11 +2915,17 @@ static int ext4_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
return err;
}
-struct ext4_dir_entry_2 *ext4_init_dot_dotdot(struct inode *inode,
- struct ext4_dir_entry_2 *de,
- int blocksize, int csum_size,
- unsigned int parent_ino, int dotdot_real_len)
+int ext4_init_dirblock(handle_t *handle, struct inode *inode,
+ struct buffer_head *bh, unsigned int parent_ino,
+ void *inline_buf, int inline_size)
{
+ struct ext4_dir_entry_2 *de = (struct ext4_dir_entry_2 *) bh->b_data;
+ size_t blocksize = bh->b_size;
+ int csum_size = 0, header_size;
+
+ if (ext4_has_feature_metadata_csum(inode->i_sb))
+ csum_size = sizeof(struct ext4_dir_entry_tail);
+
de->inode = cpu_to_le32(inode->i_ino);
de->name_len = 1;
de->rec_len = ext4_rec_len_to_disk(ext4_dir_rec_len(de->name_len, NULL),
@@ -2930,18 +2936,29 @@ struct ext4_dir_entry_2 *ext4_init_dot_dotdot(struct inode *inode,
de = ext4_next_entry(de, blocksize);
de->inode = cpu_to_le32(parent_ino);
de->name_len = 2;
- if (!dotdot_real_len)
- de->rec_len = ext4_rec_len_to_disk(blocksize -
- (csum_size + ext4_dir_rec_len(1, NULL)),
- blocksize);
- else
+ memcpy(de->name, "..", 3);
+ ext4_set_de_type(inode->i_sb, de, S_IFDIR);
+ if (inline_buf) {
de->rec_len = ext4_rec_len_to_disk(
ext4_dir_rec_len(de->name_len, NULL),
blocksize);
- memcpy(de->name, "..", 3);
- ext4_set_de_type(inode->i_sb, de, S_IFDIR);
+ de = ext4_next_entry(de, blocksize);
+ header_size = (char *)de - bh->b_data;
+ memcpy((void *)de, inline_buf, inline_size);
+ ext4_update_final_de(bh->b_data, inline_size + header_size,
+ blocksize - csum_size);
+ } else {
+ de->rec_len = ext4_rec_len_to_disk(blocksize -
+ (csum_size + ext4_dir_rec_len(1, NULL)),
+ blocksize);
+ }
- return ext4_next_entry(de, blocksize);
+ if (csum_size)
+ ext4_initialize_dirent_tail(bh, blocksize);
+ BUFFER_TRACE(dir_block, "call ext4_handle_dirty_metadata");
+ set_buffer_uptodate(bh);
+ set_buffer_verified(bh);
+ return ext4_handle_dirty_dirblock(handle, inode, bh);
}
int ext4_init_new_dir(handle_t *handle, struct inode *dir,
@@ -2950,13 +2967,8 @@ int ext4_init_new_dir(handle_t *handle, struct inode *dir,
struct buffer_head *dir_block = NULL;
struct ext4_dir_entry_2 *de;
ext4_lblk_t block = 0;
- unsigned int blocksize = dir->i_sb->s_blocksize;
- int csum_size = 0;
int err;
- if (ext4_has_feature_metadata_csum(dir->i_sb))
- csum_size = sizeof(struct ext4_dir_entry_tail);
-
if (ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) {
err = ext4_try_create_inline_dir(handle, dir, inode);
if (err < 0 && err != -ENOSPC)
@@ -2965,21 +2977,15 @@ int ext4_init_new_dir(handle_t *handle, struct inode *dir,
goto out;
}
+ set_nlink(inode, 2);
inode->i_size = 0;
dir_block = ext4_append(handle, inode, &block);
if (IS_ERR(dir_block))
return PTR_ERR(dir_block);
de = (struct ext4_dir_entry_2 *)dir_block->b_data;
- ext4_init_dot_dotdot(inode, de, blocksize, csum_size, dir->i_ino, 0);
- set_nlink(inode, 2);
- if (csum_size)
- ext4_initialize_dirent_tail(dir_block, blocksize);
-
- BUFFER_TRACE(dir_block, "call ext4_handle_dirty_metadata");
- err = ext4_handle_dirty_dirblock(handle, inode, dir_block);
+ err = ext4_init_dirblock(handle, inode, dir_block, dir->i_ino, NULL, 0);
if (err)
goto out;
- set_buffer_verified(dir_block);
out:
brelse(dir_block);
return err;
--
2.47.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] ext4: refactor the inline directory conversion and new directory codepaths
2025-07-12 18:12 ` [PATCH 3/3] ext4: refactor the inline directory conversion and new directory codepaths Theodore Ts'o
@ 2025-07-12 21:12 ` kernel test robot
2025-07-21 23:15 ` Eric Biggers
0 siblings, 1 reply; 8+ messages in thread
From: kernel test robot @ 2025-07-12 21:12 UTC (permalink / raw)
To: Theodore Ts'o, Ext4 Developers List
Cc: oe-kbuild-all, linux-hardening, ethan, Theodore Ts'o
Hi Theodore,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on linus/master v6.16-rc5 next-20250711]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Theodore-Ts-o/ext4-use-memcpy-instead-of-strcpy/20250713-021635
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/r/20250712181249.434530-3-tytso%40mit.edu
patch subject: [PATCH 3/3] ext4: refactor the inline directory conversion and new directory codepaths
config: i386-buildonly-randconfig-004-20250713 (https://download.01.org/0day-ci/archive/20250713/202507130429.rPIzofCD-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250713/202507130429.rPIzofCD-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/202507130429.rPIzofCD-lkp@intel.com/
All warnings (new ones prefixed by >>):
fs/ext4/namei.c: In function 'ext4_init_new_dir':
>> fs/ext4/namei.c:2968:34: warning: variable 'de' set but not used [-Wunused-but-set-variable]
2968 | struct ext4_dir_entry_2 *de;
| ^~
vim +/de +2968 fs/ext4/namei.c
a774f9c20e0864 Tao Ma 2012-12-10 2963
8016e29f4362e2 Harshad Shirwadkar 2020-10-15 2964 int ext4_init_new_dir(handle_t *handle, struct inode *dir,
a774f9c20e0864 Tao Ma 2012-12-10 2965 struct inode *inode)
ac27a0ec112a08 Dave Kleikamp 2006-10-11 2966 {
dabd991f9d8e32 Namhyung Kim 2011-01-10 2967 struct buffer_head *dir_block = NULL;
617ba13b31fbf5 Mingming Cao 2006-10-11 @2968 struct ext4_dir_entry_2 *de;
dc6982ff4db1f4 Theodore Ts'o 2013-02-14 2969 ext4_lblk_t block = 0;
a774f9c20e0864 Tao Ma 2012-12-10 2970 int err;
ac27a0ec112a08 Dave Kleikamp 2006-10-11 2971
3c47d54170b6a6 Tao Ma 2012-12-10 2972 if (ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) {
3c47d54170b6a6 Tao Ma 2012-12-10 2973 err = ext4_try_create_inline_dir(handle, dir, inode);
3c47d54170b6a6 Tao Ma 2012-12-10 2974 if (err < 0 && err != -ENOSPC)
3c47d54170b6a6 Tao Ma 2012-12-10 2975 goto out;
3c47d54170b6a6 Tao Ma 2012-12-10 2976 if (!err)
3c47d54170b6a6 Tao Ma 2012-12-10 2977 goto out;
3c47d54170b6a6 Tao Ma 2012-12-10 2978 }
3c47d54170b6a6 Tao Ma 2012-12-10 2979
5eb8361f6b02c3 Theodore Ts'o 2025-07-12 2980 set_nlink(inode, 2);
dc6982ff4db1f4 Theodore Ts'o 2013-02-14 2981 inode->i_size = 0;
0f70b40613ee14 Theodore Ts'o 2013-02-15 2982 dir_block = ext4_append(handle, inode, &block);
0f70b40613ee14 Theodore Ts'o 2013-02-15 2983 if (IS_ERR(dir_block))
0f70b40613ee14 Theodore Ts'o 2013-02-15 2984 return PTR_ERR(dir_block);
a774f9c20e0864 Tao Ma 2012-12-10 2985 de = (struct ext4_dir_entry_2 *)dir_block->b_data;
5eb8361f6b02c3 Theodore Ts'o 2025-07-12 2986 err = ext4_init_dirblock(handle, inode, dir_block, dir->i_ino, NULL, 0);
a774f9c20e0864 Tao Ma 2012-12-10 2987 if (err)
a774f9c20e0864 Tao Ma 2012-12-10 2988 goto out;
a774f9c20e0864 Tao Ma 2012-12-10 2989 out:
a774f9c20e0864 Tao Ma 2012-12-10 2990 brelse(dir_block);
a774f9c20e0864 Tao Ma 2012-12-10 2991 return err;
a774f9c20e0864 Tao Ma 2012-12-10 2992 }
a774f9c20e0864 Tao Ma 2012-12-10 2993
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] ext4: replace strcmp with direct comparison for '.' and '..'
2025-07-12 18:12 [PATCH 1/3] ext4: replace strcmp with direct comparison for '.' and '..' Theodore Ts'o
2025-07-12 18:12 ` [PATCH 2/3] ext4: use memcpy() instead of strcpy() Theodore Ts'o
2025-07-12 18:12 ` [PATCH 3/3] ext4: refactor the inline directory conversion and new directory codepaths Theodore Ts'o
@ 2025-07-19 21:45 ` Theodore Ts'o
2 siblings, 0 replies; 8+ messages in thread
From: Theodore Ts'o @ 2025-07-19 21:45 UTC (permalink / raw)
To: Ext4 Developers List, Theodore Ts'o; +Cc: linux-hardening, ethan
On Sat, 12 Jul 2025 14:12:47 -0400, Theodore Ts'o wrote:
> In a discussion over a proposed patch, "ext4: replace strcpy() with
> '.' assignment"[1], I had asserted that directory entries in ext4 were
> not NUL terminated, and hence it was safe to replace strcpy() with a
> direct assignment. As it turns out, this was incorrect. It's true
> for all all directory entries *except* for '.' and '..' where the
> kernel was using strcmp() and where e2fsck actually checks and offers
> to fix things if '.' and '..' are not NUL terminated.
>
> [...]
Applied, thanks!
[1/3] ext4: replace strcmp with direct comparison for '.' and '..'
commit: 3658b8b3398eb2a49ee8d1ac88e5cdc41764f1c9
[2/3] ext4: use memcpy() instead of strcpy()
commit: a35454ecf8a320c49954fdcdae0e8d3323067632
[3/3] ext4: refactor the inline directory conversion and new directory codepaths
commit: 90f097b1403f232a202c501bfd49b1b196e840ea
Best regards,
--
Theodore Ts'o <tytso@mit.edu>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] ext4: refactor the inline directory conversion and new directory codepaths
2025-07-12 21:12 ` kernel test robot
@ 2025-07-21 23:15 ` Eric Biggers
2025-07-30 15:01 ` Andy Shevchenko
0 siblings, 1 reply; 8+ messages in thread
From: Eric Biggers @ 2025-07-21 23:15 UTC (permalink / raw)
To: Theodore Ts'o
Cc: kernel test robot, Ext4 Developers List, oe-kbuild-all,
linux-hardening, ethan
Hi Ted,
On Sun, Jul 13, 2025 at 05:12:55AM +0800, kernel test robot wrote:
> All warnings (new ones prefixed by >>):
>
> fs/ext4/namei.c: In function 'ext4_init_new_dir':
> >> fs/ext4/namei.c:2968:34: warning: variable 'de' set but not used [-Wunused-but-set-variable]
> 2968 | struct ext4_dir_entry_2 *de;
> | ^~
This warning is present in ext4/dev now.
- Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] ext4: refactor the inline directory conversion and new directory codepaths
2025-07-21 23:15 ` Eric Biggers
@ 2025-07-30 15:01 ` Andy Shevchenko
0 siblings, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2025-07-30 15:01 UTC (permalink / raw)
To: Eric Biggers
Cc: Theodore Ts'o, kernel test robot, Ext4 Developers List,
oe-kbuild-all, linux-hardening, ethan
On Mon, Jul 21, 2025 at 04:15:59PM -0700, Eric Biggers wrote:
> On Sun, Jul 13, 2025 at 05:12:55AM +0800, kernel test robot wrote:
> > All warnings (new ones prefixed by >>):
> >
> > fs/ext4/namei.c: In function 'ext4_init_new_dir':
> > >> fs/ext4/namei.c:2968:34: warning: variable 'de' set but not used [-Wunused-but-set-variable]
> > 2968 | struct ext4_dir_entry_2 *de;
> > | ^~
>
> This warning is present in ext4/dev now.
For somebody (who doesn't alter CONFIG_WERROR) this is a build breakage with both GCC and clang.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] ext4: use memcpy() instead of strcpy()
2025-07-12 18:12 ` [PATCH 2/3] ext4: use memcpy() instead of strcpy() Theodore Ts'o
@ 2025-07-30 15:02 ` Andy Shevchenko
0 siblings, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2025-07-30 15:02 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List, linux-hardening, ethan
On Sat, Jul 12, 2025 at 02:12:48PM -0400, Theodore Ts'o wrote:
> The strcpy() function is considered dangerous and eeeevil by people
> who are using sophisticated code analysis tools such as "grep". This
> is true even when a quick inspection would show that the source is a
> constant string ("." or "..") and the destination is a fixed array
> which is guaranteed to have enough space. Make the "grep" code
> analysis tool happy by using memcpy() isstead of strcpy(). :-)
Why simple 2-arg strscpy() can't be used?
...
> - strcpy(fake.name, ".");
> + memcpy(fake.name, ".", 2);
s/strcpy/strscpy/
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-07-30 14:02 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-12 18:12 [PATCH 1/3] ext4: replace strcmp with direct comparison for '.' and '..' Theodore Ts'o
2025-07-12 18:12 ` [PATCH 2/3] ext4: use memcpy() instead of strcpy() Theodore Ts'o
2025-07-30 15:02 ` Andy Shevchenko
2025-07-12 18:12 ` [PATCH 3/3] ext4: refactor the inline directory conversion and new directory codepaths Theodore Ts'o
2025-07-12 21:12 ` kernel test robot
2025-07-21 23:15 ` Eric Biggers
2025-07-30 15:01 ` Andy Shevchenko
2025-07-19 21:45 ` [PATCH 1/3] ext4: replace strcmp with direct comparison for '.' and '..' Theodore Ts'o
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.