* [PATCH v13 01/11] ext4: validate count against limit in ext4_dx_csum_verify/_set
2026-10-29 12:23 [PATCH v13 00/11] Data in direntry (dirdata) feature Artem Blagodarenko
@ 2026-10-29 12:24 ` Artem Blagodarenko
2026-10-29 12:37 ` sashiko-bot
2026-10-29 12:24 ` [PATCH v13 02/11] ext4: replace ext4_dir_entry with ext4_dir_entry_2 Artem Blagodarenko
` (9 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Artem Blagodarenko @ 2026-10-29 12:24 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, jack, adilger.kernel, ablagodarenko, xiaowu.417,
Artem Blagodarenko
dx_countlimit's count field was read from disk and used directly to
compute the checksummed range (count_offset + count * sizeof(dx_entry))
without ever being checked against limit -- only limit itself was
bounds-checked against the block size. A corrupted or maliciously
crafted filesystem image that sets count to a large value (e.g. 65535)
makes ext4_chksum() read far past the end of the directory block
buffer, hitting adjacent slab objects.
Reported-by: xiaowu.417@qq.com
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
---
fs/ext4/namei.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 231a274b814e..76b9aa3fb03c 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -477,6 +477,10 @@ static int ext4_dx_csum_verify(struct inode *inode,
warn_no_space_for_csum(inode);
return 0;
}
+ if (count > limit) {
+ EXT4_ERROR_INODE(inode, "dir seems corrupt? Run e2fsck -D.");
+ return 0;
+ }
t = (struct dx_tail *)(((struct dx_entry *)c) + limit);
if (t->dt_checksum != ext4_dx_csum(inode, dirent, count_offset,
@@ -506,6 +510,10 @@ static void ext4_dx_csum_set(struct inode *inode, struct ext4_dir_entry *dirent)
warn_no_space_for_csum(inode);
return;
}
+ if (count > limit) {
+ EXT4_ERROR_INODE(inode, "dir seems corrupt? Run e2fsck -D.");
+ return;
+ }
t = (struct dx_tail *)(((struct dx_entry *)c) + limit);
t->dt_checksum = ext4_dx_csum(inode, dirent, count_offset, count, t);
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v13 02/11] ext4: replace ext4_dir_entry with ext4_dir_entry_2
2026-10-29 12:23 [PATCH v13 00/11] Data in direntry (dirdata) feature Artem Blagodarenko
2026-10-29 12:24 ` [PATCH v13 01/11] ext4: validate count against limit in ext4_dx_csum_verify/_set Artem Blagodarenko
@ 2026-10-29 12:24 ` Artem Blagodarenko
2026-10-29 12:34 ` sashiko-bot
2026-10-29 12:24 ` [PATCH v13 03/11] ext4: add ext4_dir_entry_is_tail() Artem Blagodarenko
` (8 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Artem Blagodarenko @ 2026-10-29 12:24 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, jack, adilger.kernel, ablagodarenko, Artem Blagodarenko,
Andreas Dilger
From: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Replace remaining uses of struct ext4_dir_entry in namei.c
with struct ext4_dir_entry_2.
The code paths affected by this change already depend on the
filetype feature, so using struct ext4_dir_entry_2 is
appropriate and avoids mixing the two directory entry types
unnecessarily.
This change does not affect support for 16-bit rec_len.
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
---
fs/ext4/ext4.h | 6 ++----
fs/ext4/namei.c | 38 +++++++++++++++++++-------------------
2 files changed, 21 insertions(+), 23 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 8d3af062a830..16df13874559 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2507,10 +2507,8 @@ static inline int ext4_emergency_state(struct super_block *sb)
* Structure of a directory entry
*/
#define EXT4_NAME_LEN 255
-/*
- * Base length of the ext4 directory entry excluding the name length
- */
-#define EXT4_BASE_DIR_LEN (sizeof(struct ext4_dir_entry_2) - EXT4_NAME_LEN)
+/* offsetof avoids sizeof()+padding giving 9 instead of the correct on-disk 8 */
+#define EXT4_BASE_DIR_LEN offsetof(struct ext4_dir_entry_2, name)
struct ext4_dir_entry {
__le32 inode; /* Inode number */
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 76b9aa3fb03c..ec23e72cc422 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -102,7 +102,7 @@ static struct buffer_head *ext4_append(handle_t *handle,
}
static int ext4_dx_csum_verify(struct inode *inode,
- struct ext4_dir_entry *dirent);
+ struct ext4_dir_entry_2 *dirent);
/*
* Hints to ext4_read_dirblock regarding whether we expect a directory
@@ -128,7 +128,7 @@ static struct buffer_head *__ext4_read_dirblock(struct inode *inode,
unsigned int line)
{
struct buffer_head *bh;
- struct ext4_dir_entry *dirent;
+ struct ext4_dir_entry_2 *dirent;
int is_dx_block = 0;
if (block >= inode->i_size >> inode->i_blkbits) {
@@ -160,7 +160,7 @@ static struct buffer_head *__ext4_read_dirblock(struct inode *inode,
}
if (!bh)
return NULL;
- dirent = (struct ext4_dir_entry *) bh->b_data;
+ dirent = (struct ext4_dir_entry_2 *) bh->b_data;
/* Determine whether or not we have an index block */
if (is_dx(inode)) {
if (block == 0)
@@ -317,13 +317,13 @@ static struct ext4_dir_entry_tail *get_dirent_tail(struct inode *inode,
int blocksize = EXT4_BLOCK_SIZE(inode->i_sb);
#ifdef PARANOID
- struct ext4_dir_entry *d, *top;
+ struct ext4_dir_entry_2 *d, *top;
- d = (struct ext4_dir_entry *)bh->b_data;
- top = (struct ext4_dir_entry *)(bh->b_data +
+ d = (struct ext4_dir_entry_2 *)bh->b_data;
+ top = (struct ext4_dir_entry_2 *)(bh->b_data +
(blocksize - sizeof(struct ext4_dir_entry_tail)));
while (d < top && ext4_rec_len_from_disk(d->rec_len, blocksize))
- d = (struct ext4_dir_entry *)(((void *)d) +
+ d = (struct ext4_dir_entry_2 *)(((void *)d) +
ext4_rec_len_from_disk(d->rec_len, blocksize));
if (d != top)
@@ -410,22 +410,22 @@ int ext4_handle_dirty_dirblock(handle_t *handle,
}
static struct dx_countlimit *get_dx_countlimit(struct inode *inode,
- struct ext4_dir_entry *dirent,
+ struct ext4_dir_entry_2 *dirent,
int *offset)
{
- struct ext4_dir_entry *dp;
+ struct ext4_dir_entry_2 *de;
struct dx_root_info *root;
int count_offset;
int blocksize = EXT4_BLOCK_SIZE(inode->i_sb);
unsigned int rlen = ext4_rec_len_from_disk(dirent->rec_len, blocksize);
if (rlen == blocksize)
- count_offset = 8;
+ count_offset = sizeof(struct dx_node);
else if (rlen == 12) {
- dp = (struct ext4_dir_entry *)(((void *)dirent) + 12);
- if (ext4_rec_len_from_disk(dp->rec_len, blocksize) != blocksize - 12)
+ de = (struct ext4_dir_entry_2 *)(((void *)dirent) + 12);
+ if (ext4_rec_len_from_disk(de->rec_len, blocksize) != blocksize - 12)
return NULL;
- root = (struct dx_root_info *)(((void *)dp + 12));
+ root = (struct dx_root_info *)(((void *)de + 12));
if (root->reserved_zero ||
root->info_length != sizeof(struct dx_root_info))
return NULL;
@@ -438,7 +438,7 @@ static struct dx_countlimit *get_dx_countlimit(struct inode *inode,
return (struct dx_countlimit *)(((void *)dirent) + count_offset);
}
-static __le32 ext4_dx_csum(struct inode *inode, struct ext4_dir_entry *dirent,
+static __le32 ext4_dx_csum(struct inode *inode, struct ext4_dir_entry_2 *dirent,
int count_offset, int count, struct dx_tail *t)
{
struct ext4_inode_info *ei = EXT4_I(inode);
@@ -456,7 +456,7 @@ static __le32 ext4_dx_csum(struct inode *inode, struct ext4_dir_entry *dirent,
}
static int ext4_dx_csum_verify(struct inode *inode,
- struct ext4_dir_entry *dirent)
+ struct ext4_dir_entry_2 *dirent)
{
struct dx_countlimit *c;
struct dx_tail *t;
@@ -489,7 +489,7 @@ static int ext4_dx_csum_verify(struct inode *inode,
return 1;
}
-static void ext4_dx_csum_set(struct inode *inode, struct ext4_dir_entry *dirent)
+static void ext4_dx_csum_set(struct inode *inode, struct ext4_dir_entry_2 *dirent)
{
struct dx_countlimit *c;
struct dx_tail *t;
@@ -523,7 +523,7 @@ static inline int ext4_handle_dirty_dx_node(handle_t *handle,
struct inode *inode,
struct buffer_head *bh)
{
- ext4_dx_csum_set(inode, (struct ext4_dir_entry *)bh->b_data);
+ ext4_dx_csum_set(inode, (struct ext4_dir_entry_2 *)bh->b_data);
return ext4_handle_dirty_metadata(handle, inode, bh);
}
@@ -1498,7 +1498,7 @@ int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size,
}
static int is_dx_internal_node(struct inode *dir, ext4_lblk_t block,
- struct ext4_dir_entry *de)
+ struct ext4_dir_entry_2 *de)
{
struct super_block *sb = dir->i_sb;
@@ -1629,7 +1629,7 @@ static struct buffer_head *__ext4_find_entry(struct inode *dir,
}
if (!buffer_verified(bh) &&
!is_dx_internal_node(dir, block,
- (struct ext4_dir_entry *)bh->b_data) &&
+ (struct ext4_dir_entry_2 *)bh->b_data) &&
!ext4_dirblock_csum_verify(dir, bh)) {
EXT4_ERROR_INODE_ERR(dir, EFSBADCRC,
"checksumming directory "
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v13 03/11] ext4: add ext4_dir_entry_is_tail()
2026-10-29 12:23 [PATCH v13 00/11] Data in direntry (dirdata) feature Artem Blagodarenko
2026-10-29 12:24 ` [PATCH v13 01/11] ext4: validate count against limit in ext4_dx_csum_verify/_set Artem Blagodarenko
2026-10-29 12:24 ` [PATCH v13 02/11] ext4: replace ext4_dir_entry with ext4_dir_entry_2 Artem Blagodarenko
@ 2026-10-29 12:24 ` Artem Blagodarenko
2026-10-29 12:31 ` sashiko-bot
2026-10-29 12:24 ` [PATCH v13 04/11] ext4: add dirdata format definitions and access helpers Artem Blagodarenko
` (7 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Artem Blagodarenko @ 2026-10-29 12:24 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, jack, adilger.kernel, ablagodarenko, Artem Blagodarenko,
Andreas Dilger
From: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Replace open-coded checks for directory tail entries with a call
to ext4_dir_entry_is_tail(). This helper will also be used by
upcoming changes.
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
---
fs/ext4/ext4.h | 16 ++++++++++++++++
fs/ext4/namei.c | 14 ++++++--------
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 16df13874559..41152914aa6c 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -4101,6 +4101,22 @@ static inline void ext4_clear_io_unwritten_flag(ext4_io_end_t *io_end)
io_end->flag &= ~EXT4_IO_END_UNWRITTEN;
}
+/*
+ * ext4_dir_entry_is_tail() - Check if a directory entry is a tail entry.
+ * @de: directory entry to check
+ *
+ * Returns true if @de is a directory block tail entry (checksum record).
+ */
+static inline bool ext4_dir_entry_is_tail(struct ext4_dir_entry_2 *de)
+{
+ struct ext4_dir_entry_tail *t = (struct ext4_dir_entry_tail *)de;
+
+ return !t->det_reserved_zero1 &&
+ le16_to_cpu(t->det_rec_len) == sizeof(*t) &&
+ !t->det_reserved_zero2 &&
+ t->det_reserved_ft == EXT4_FT_DIR_CSUM;
+}
+
extern const struct iomap_ops ext4_iomap_ops;
extern const struct iomap_ops ext4_iomap_report_ops;
extern const struct iomap_ops ext4_iomap_buffered_write_ops;
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index ec23e72cc422..533340c5dbc0 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -331,14 +331,10 @@ static struct ext4_dir_entry_tail *get_dirent_tail(struct inode *inode,
t = (struct ext4_dir_entry_tail *)d;
#else
- t = EXT4_DIRENT_TAIL(bh->b_data, EXT4_BLOCK_SIZE(inode->i_sb));
+ t = EXT4_DIRENT_TAIL(bh->b_data, blocksize);
#endif
- if (t->det_reserved_zero1 ||
- (ext4_rec_len_from_disk(t->det_rec_len, blocksize) !=
- sizeof(struct ext4_dir_entry_tail)) ||
- t->det_reserved_zero2 ||
- t->det_reserved_ft != EXT4_FT_DIR_CSUM)
+ if (!ext4_dir_entry_is_tail((struct ext4_dir_entry_2 *)t))
return NULL;
return t;
@@ -478,7 +474,8 @@ static int ext4_dx_csum_verify(struct inode *inode,
return 0;
}
if (count > limit) {
- EXT4_ERROR_INODE(inode, "dir seems corrupt? Run e2fsck -D.");
+ EXT4_ERROR_INODE(inode, "dir seems corrupt (%i > %i)? "
+ "Run e2fsck -D.", count, limit);
return 0;
}
t = (struct dx_tail *)(((struct dx_entry *)c) + limit);
@@ -511,7 +508,8 @@ static void ext4_dx_csum_set(struct inode *inode, struct ext4_dir_entry_2 *diren
return;
}
if (count > limit) {
- EXT4_ERROR_INODE(inode, "dir seems corrupt? Run e2fsck -D.");
+ EXT4_ERROR_INODE(inode, "dir seems corrupt (%i > %i)? "
+ " Run e2fsck -D.", count, limit);
return;
}
t = (struct dx_tail *)(((struct dx_entry *)c) + limit);
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v13 04/11] ext4: add dirdata format definitions and access helpers
2026-10-29 12:23 [PATCH v13 00/11] Data in direntry (dirdata) feature Artem Blagodarenko
` (2 preceding siblings ...)
2026-10-29 12:24 ` [PATCH v13 03/11] ext4: add ext4_dir_entry_is_tail() Artem Blagodarenko
@ 2026-10-29 12:24 ` Artem Blagodarenko
2026-10-29 12:31 ` sashiko-bot
2026-10-29 12:24 ` [PATCH v13 05/11] ext4: refactor dx_root to support variable dirent sizes Artem Blagodarenko
` (6 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Artem Blagodarenko @ 2026-10-29 12:24 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, jack, adilger.kernel, ablagodarenko, Pravin Shelar,
Artem Blagodarenko, Andreas Dilger
Define the on-disk format for ext4 directory entry extension data.
The upper four bits of de->file_type indicate the presence of
optional data stored after the filename NUL terminator. This patch
defines flags for LUFID, 64-bit inode numbers, and casefold hash
data stored in that area.
Add struct ext4_dirent_data_header to describe variable-length
extension records and struct ext4_dirent_hash for hash storage used
by casefold and fscrypt.
Provide ext4_dirdata_next() to advance to the next extension record
and ext4_dirent_get_data_len() to compute the total extension data
length associated with a directory entry.
No functional changes.
Signed-off-by: Pravin Shelar <pravin.shelar@sun.com>
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
---
fs/ext4/ext4.h | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 117 insertions(+)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 41152914aa6c..c79295415c07 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2589,6 +2589,49 @@ struct ext4_dir_entry_tail {
#define EXT4_FT_SYMLINK 7
#define EXT4_FT_MAX 8
+#define EXT4_FT_MASK 0xf
+
+#if EXT4_FT_MAX > EXT4_FT_MASK
+#error "conflicting EXT4_FT_MAX and EXT4_FT_MASK"
+#endif
+
+/*
+ * d_type has 4 unused bits, so it can hold four types of data. These different
+ * types of data (e.g. fscypt hash, high 32 bits of 64-bit inode number) can be
+ * stored, in flag order, after file-name in ext4 dirent.
+ *
+ * These flags are added to d_type if ext4 dirent has extra data after
+ * filename. This data length is variable and length is stored in first byte
+ * of data. Data starts after filename NUL byte.
+ */
+#define EXT4_DIRENT_LUFID 0x10
+#define EXT4_DIRENT_INO64 0x20
+#define EXT4_DIRENT_CFHASH 0x40
+
+struct ext4_fid {
+ char fid[16]; /* 128-bit unique file identifier */
+};
+
+struct ext4_dirent_data_header {
+ /* length of this header + the whole data blob */
+ __u8 ddh_length;
+} __packed;
+
+struct ext4_dirent_fid {
+ struct ext4_dirent_data_header df_header;
+ struct ext4_fid df_fid[];
+};
+
+#define EXT4_LUFID_MAGIC 0xAD200907UL
+struct ext4_dentry_param {
+ __u32 edp_magic; /* EXT4_LUFID_MAGIC */
+ __u8 edp_dfid[]; /* struct ext4_dirent_fid payload */
+};
+
+struct ext4_dirent_hash {
+ struct ext4_dirent_data_header dh_header;
+ struct ext4_dir_entry_hash dh_hash;
+} __packed;
#define EXT4_FT_DIR_CSUM 0xDE
@@ -4117,6 +4160,80 @@ static inline bool ext4_dir_entry_is_tail(struct ext4_dir_entry_2 *de)
t->det_reserved_ft == EXT4_FT_DIR_CSUM;
}
+/*
+ * Advance to the next dirdata record header starting from @ddh.
+ *
+ * Uses READ_ONCE() on ddh_length to prevent the compiler from re-fetching this
+ * on-disk field and to guard against a concurrent write racing with any bounds
+ * check the caller performs on the returned pointer.
+ */
+static inline struct ext4_dirent_data_header *
+ext4_dirdata_next(struct ext4_dirent_data_header *ddh)
+{
+ return (struct ext4_dirent_data_header *)
+ ((char *)ddh + READ_ONCE(ddh->ddh_length));
+}
+
+/*
+ * ext4_dirent_get_data_len() - Compute the total dirdata length for an entry.
+ * @de: directory entry
+ * @rec_len: the record length of the directory entry (decoded)
+ *
+ * Computes the length of optional data stored after the filename (and its
+ * implicit NUL terminator). Each extension is indicated by a bit in the
+ * high 4 bits of de->file_type; the first byte of each extension is its
+ * length (including that length byte itself).
+ *
+ * Returns 0 for tail entries and for entries with no dirdata.
+ */
+static inline int ext4_dirent_get_data_len(struct ext4_dir_entry_2 *de,
+ unsigned int rec_len)
+{
+ __u8 extra_data_flags;
+ struct ext4_dirent_data_header *ddh;
+ int dlen = 0;
+ unsigned int offset;
+
+ if (ext4_dir_entry_is_tail(de))
+ return 0;
+
+ extra_data_flags = (de->file_type & ~EXT4_FT_MASK) >> 4;
+ /* offset from start of entry to after filename + NUL */
+ offset = EXT4_BASE_DIR_LEN + de->name_len + 1;
+
+ /* bounds check: ensure we start reading within the entry */
+ if (offset >= rec_len)
+ return 0;
+
+ ddh = (struct ext4_dirent_data_header *)((char *)de + offset);
+
+ while (extra_data_flags) {
+ if (extra_data_flags & 1) {
+ /* bounds check before reading ddh_length */
+ if (offset + sizeof(*ddh) > rec_len)
+ return 0;
+
+ /* Use READ_ONCE to prevent the compiler from re-fetching
+ * this disk field and to guard against a concurrent
+ * write racing with our bounds check below (TOCTOU). */
+ {
+ u8 ddh_len = READ_ONCE(ddh->ddh_length);
+
+ if (ddh_len == 0 || ddh_len > rec_len - offset)
+ return 0;
+ dlen += ddh_len + (dlen == 0);
+ offset += ddh_len;
+ /* advance using the already-validated local copy
+ * to avoid re-fetching ddh->ddh_length */
+ ddh = (struct ext4_dirent_data_header *)
+ ((char *)ddh + ddh_len);
+ }
+ }
+ extra_data_flags >>= 1;
+ }
+ return dlen;
+}
+
extern const struct iomap_ops ext4_iomap_ops;
extern const struct iomap_ops ext4_iomap_report_ops;
extern const struct iomap_ops ext4_iomap_buffered_write_ops;
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v13 05/11] ext4: refactor dx_root to support variable dirent sizes
2026-10-29 12:23 [PATCH v13 00/11] Data in direntry (dirdata) feature Artem Blagodarenko
` (3 preceding siblings ...)
2026-10-29 12:24 ` [PATCH v13 04/11] ext4: add dirdata format definitions and access helpers Artem Blagodarenko
@ 2026-10-29 12:24 ` Artem Blagodarenko
2026-10-29 12:49 ` sashiko-bot
2026-10-29 12:24 ` [PATCH v13 06/11] ext4: add ext4_dir_entry_len() Artem Blagodarenko
` (5 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Artem Blagodarenko @ 2026-10-29 12:24 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, jack, adilger.kernel, ablagodarenko, Artem Blagodarenko,
Pravin Shelar, Andreas Dilger
From: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Split monolithic definition of dx_root struct to separate dx_root_info
from fake struct ext4_dir_entry2 for improved code readability.
This allows "." and ".." dirents to have different sizes if necessary,
since we can't assume the rec_len 12 if dx_root dirents have dirdata.
Adds dx_get_dx_info() accessor instead of complex typecast at callers.
Does not change any functionality.
Also add the ext4_dir_entry_len() helper, which dx_get_dx_info() uses
to step over the "." and ".." entries now that they may carry dirdata.
Signed-off-by: Pravin Shelar <pravin.shelar@sun.com>
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
---
fs/ext4/ext4.h | 26 ++++++
fs/ext4/namei.c | 211 +++++++++++++++++++++++++++++-------------------
2 files changed, 154 insertions(+), 83 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index c79295415c07..edb95cbc4439 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -4234,6 +4234,32 @@ static inline int ext4_dirent_get_data_len(struct ext4_dir_entry_2 *de,
return dlen;
}
+/*
+ * ext4_dir_entry_len() - Compute the required rec_len for a directory entry.
+ * @de: directory entry (used to read name_len and any dirdata length)
+ * @blocksize: size of the buffer @de lives in (the real directory block
+ * size, or the smaller inline-data buffer size for inline
+ * directories) -- used only to decode @de->rec_len's "0/65535
+ * means rest of buffer" sentinel correctly.
+ * @dir: directory inode (may be NULL for '.' and '..' entries, which
+ * never carry the casefold+fscrypt hash regardless of the
+ * directory's feature flags)
+ *
+ * Returns the minimum record length needed to hold @de, rounded up to the
+ * directory alignment and including room for the casefold+fscrypt hash if
+ * the directory requires it.
+ */
+static inline unsigned int ext4_dir_entry_len(struct ext4_dir_entry_2 *de,
+ unsigned int blocksize,
+ const struct inode *dir)
+{
+ unsigned int rec_len = ext4_rec_len_from_disk(de->rec_len, blocksize);
+ unsigned int dirdata = ext4_dirent_get_data_len(de, rec_len);
+
+ return ext4_dir_rec_len(de->name_len + dirdata, dir);
+}
+
+
extern const struct iomap_ops ext4_iomap_ops;
extern const struct iomap_ops ext4_iomap_report_ops;
extern const struct iomap_ops ext4_iomap_buffered_write_ops;
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 533340c5dbc0..59cc01b91c41 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -244,22 +244,13 @@ struct dx_entry
* hash version mod 4 should never be 0. Sincerely, the paranoia department.
*/
-struct dx_root
+struct dx_root_info
{
- struct fake_dirent dot;
- char dot_name[4];
- struct fake_dirent dotdot;
- char dotdot_name[4];
- struct dx_root_info
- {
- __le32 reserved_zero;
- u8 hash_version;
- u8 info_length; /* 8 */
- u8 indirect_levels;
- u8 unused_flags;
- }
- info;
- struct dx_entry entries[];
+ __le32 reserved_zero;
+ u8 hash_version;
+ u8 info_length; /* 8 */
+ u8 indirect_levels;
+ u8 unused_flags;
};
struct dx_node
@@ -539,6 +530,31 @@ ext4_next_entry(struct ext4_dir_entry_2 *p, unsigned long blocksize)
* Future: use high four bits of block for coalesce-on-delete flags
* Mask them off for now.
*/
+static struct dx_root_info *dx_get_dx_info(struct inode *dir, void *de_buf)
+{
+ unsigned int blocksize = dir->i_sb->s_blocksize;
+ void *base = de_buf;
+
+ /* '.' and '..' never carry the casefold+fscrypt hash, so pass NULL
+ * for dir regardless of the directory's flags */
+ if (ext4_rec_len_from_disk(((struct ext4_dir_entry_2 *)de_buf)->rec_len,
+ blocksize) < EXT4_BASE_DIR_LEN)
+ return ERR_PTR(-EFSCORRUPTED);
+ de_buf += ext4_dir_entry_len(de_buf, blocksize, NULL);
+
+ /* dx root info is after dotdot entry */
+ if (de_buf < base || (char *)de_buf - (char *)base +
+ EXT4_BASE_DIR_LEN > blocksize)
+ return ERR_PTR(-EFSCORRUPTED);
+ de_buf += ext4_dir_entry_len(de_buf, blocksize, NULL);
+
+ if (de_buf < base || (char *)de_buf - (char *)base +
+ sizeof(struct dx_root_info) +
+ sizeof(struct dx_entry) > blocksize)
+ return ERR_PTR(-EFSCORRUPTED);
+
+ return (struct dx_root_info *)de_buf;
+}
static inline ext4_lblk_t dx_get_block(struct dx_entry *entry)
{
@@ -593,8 +609,10 @@ static inline unsigned dx_root_limit(struct inode *dir, unsigned infosize)
static inline unsigned dx_node_limit(struct inode *dir)
{
+ /* dx_node fake_dirent is always 8 bytes — it never carries a casefold
+ * hash, so pass NULL to suppress the hash-size term. */
unsigned int entry_space = dir->i_sb->s_blocksize -
- ext4_dir_rec_len(0, dir);
+ ext4_dir_rec_len(0, NULL);
if (ext4_has_feature_metadata_csum(dir->i_sb))
entry_space -= sizeof(struct dx_tail);
@@ -786,7 +804,7 @@ dx_probe(struct ext4_filename *fname, struct inode *dir,
{
unsigned count, indirect, level, i;
struct dx_entry *at, *entries, *p, *q, *m;
- struct dx_root *root;
+ struct dx_root_info *info;
struct dx_frame *frame = frame_in;
struct dx_frame *ret_err = ERR_PTR(ERR_BAD_DX_DIR);
u32 hash;
@@ -798,23 +816,31 @@ dx_probe(struct ext4_filename *fname, struct inode *dir,
if (IS_ERR(frame->bh))
return (struct dx_frame *) frame->bh;
- root = (struct dx_root *) frame->bh->b_data;
- if (root->info.hash_version != DX_HASH_TEA &&
- root->info.hash_version != DX_HASH_HALF_MD4 &&
- root->info.hash_version != DX_HASH_LEGACY &&
- root->info.hash_version != DX_HASH_SIPHASH) {
- ext4_warning_inode(dir, "Unrecognised inode hash code %u",
- root->info.hash_version);
+ info = dx_get_dx_info(dir, (struct ext4_dir_entry_2 *)frame->bh->b_data);
+ if (IS_ERR(info))
+ goto fail;
+ if (info->info_length != sizeof(struct dx_root_info)) {
+ ext4_warning_inode(dir, "corrupted htree: info_length %u",
+ info->info_length);
+ goto fail;
+ }
+ if (info->hash_version != DX_HASH_TEA &&
+ info->hash_version != DX_HASH_HALF_MD4 &&
+ info->hash_version != DX_HASH_LEGACY &&
+ info->hash_version != DX_HASH_SIPHASH) {
+ ext4_warning(dir->i_sb,
+ "Unrecognised inode hash code %d for directory #%llu",
+ info->hash_version, dir->i_ino);
goto fail;
}
if (ext4_hash_in_dirent(dir)) {
- if (root->info.hash_version != DX_HASH_SIPHASH) {
+ if (info->hash_version != DX_HASH_SIPHASH) {
ext4_warning_inode(dir,
"Hash in dirent, but hash is not SIPHASH");
goto fail;
}
} else {
- if (root->info.hash_version == DX_HASH_SIPHASH) {
+ if (info->hash_version == DX_HASH_SIPHASH) {
ext4_warning_inode(dir,
"Hash code is SIPHASH, but hash not in dirent");
goto fail;
@@ -822,7 +848,7 @@ dx_probe(struct ext4_filename *fname, struct inode *dir,
}
if (fname)
hinfo = &fname->hinfo;
- hinfo->hash_version = root->info.hash_version;
+ hinfo->hash_version = info->hash_version;
if (hinfo->hash_version <= DX_HASH_TEA)
hinfo->hash_version += EXT4_SB(dir->i_sb)->s_hash_unsigned;
hinfo->seed = EXT4_SB(dir->i_sb)->s_hash_seed;
@@ -838,13 +864,13 @@ dx_probe(struct ext4_filename *fname, struct inode *dir,
}
hash = hinfo->hash;
- if (root->info.unused_flags & 1) {
+ if (info->unused_flags & 1) {
ext4_warning_inode(dir, "Unimplemented hash flags: %#06x",
- root->info.unused_flags);
+ info->unused_flags);
goto fail;
}
- indirect = root->info.indirect_levels;
+ indirect = info->indirect_levels;
if (indirect >= ext4_dir_htree_level(dir->i_sb)) {
ext4_warning(dir->i_sb,
"Directory (ino: %llu) htree depth %#06x exceed"
@@ -857,14 +883,12 @@ dx_probe(struct ext4_filename *fname, struct inode *dir,
goto fail;
}
- entries = (struct dx_entry *)(((char *)&root->info) +
- root->info.info_length);
+ entries = (struct dx_entry *)(((char *)info) + info->info_length);
- if (dx_get_limit(entries) != dx_root_limit(dir,
- root->info.info_length)) {
+ if (dx_get_limit(entries) != dx_root_limit(dir, info->info_length)) {
ext4_warning_inode(dir, "dx entry: limit %u != root limit %u",
dx_get_limit(entries),
- dx_root_limit(dir, root->info.info_length));
+ dx_root_limit(dir, info->info_length));
goto fail;
}
@@ -941,7 +965,7 @@ dx_probe(struct ext4_filename *fname, struct inode *dir,
return ret_err;
}
-static void dx_release(struct dx_frame *frames)
+static void dx_release(struct inode *dir, struct dx_frame *frames)
{
struct dx_root_info *info;
int i;
@@ -950,7 +974,18 @@ static void dx_release(struct dx_frame *frames)
if (frames[0].bh == NULL)
return;
- info = &((struct dx_root *)frames[0].bh->b_data)->info;
+ info = dx_get_dx_info(dir, (struct ext4_dir_entry_2 *)frames[0].bh->b_data);
+ if (IS_ERR(info)) {
+ /* Cannot determine indirect_levels; release all frame buffers
+ * that may have been loaded to avoid leaking them. */
+ for (i = 0; i < EXT4_HTREE_LEVEL; i++) {
+ if (!frames[i].bh)
+ break;
+ brelse(frames[i].bh);
+ frames[i].bh = NULL;
+ }
+ return;
+ }
/* save local copy, "info" may be freed after brelse() */
indirect_levels = info->indirect_levels;
for (i = 0; i <= indirect_levels; i++) {
@@ -1256,12 +1291,12 @@ int ext4_htree_fill_tree(struct file *dir_file, __u32 start_hash,
(count && ((hashval & 1) == 0)))
break;
}
- dx_release(frames);
+ dx_release(dir, frames);
dxtrace(printk(KERN_DEBUG "Fill tree: returned %d entries, "
"next hash: %x\n", count, *next_hash));
return count;
errout:
- dx_release(frames);
+ dx_release(dir, frames);
return (err);
}
@@ -1761,7 +1796,7 @@ static struct buffer_head * ext4_dx_find_entry(struct inode *dir,
errout:
dxtrace(printk(KERN_DEBUG "%s not found\n", fname->usr_fname->name));
success:
- dx_release(frames);
+ dx_release(dir, frames);
return bh;
}
@@ -2164,44 +2199,38 @@ static int add_dirent_to_buf(handle_t *handle, struct ext4_filename *fname,
return err ? err : err2;
}
-static bool ext4_check_dx_root(struct inode *dir, struct dx_root *root)
+static bool ext4_check_dx_root(struct inode *dir,
+ struct ext4_dir_entry_2 *dot_de,
+ struct ext4_dir_entry_2 *dotdot_de,
+ struct ext4_dir_entry_2 **entry)
{
- struct fake_dirent *fde;
const char *error_msg;
- unsigned int rlen;
unsigned int blocksize = dir->i_sb->s_blocksize;
- char *blockend = (char *)root + dir->i_sb->s_blocksize;
+ struct ext4_dir_entry_2 *de = NULL;
- fde = &root->dot;
- if (unlikely(fde->name_len != 1)) {
+ if (unlikely(dot_de->name_len != 1)) {
error_msg = "invalid name_len for '.'";
goto corrupted;
}
- if (unlikely(strncmp(root->dot_name, ".", fde->name_len))) {
+ if (unlikely(strncmp(dot_de->name, ".", dot_de->name_len))) {
error_msg = "invalid name for '.'";
goto corrupted;
}
- rlen = ext4_rec_len_from_disk(fde->rec_len, blocksize);
- if (unlikely((char *)fde + rlen >= blockend)) {
- error_msg = "invalid rec_len for '.'";
- goto corrupted;
- }
- fde = &root->dotdot;
- if (unlikely(fde->name_len != 2)) {
+ if (unlikely(dotdot_de->name_len != 2)) {
error_msg = "invalid name_len for '..'";
goto corrupted;
}
- if (unlikely(strncmp(root->dotdot_name, "..", fde->name_len))) {
+ if (unlikely(strncmp(dotdot_de->name, "..", dotdot_de->name_len))) {
error_msg = "invalid name for '..'";
goto corrupted;
}
- rlen = ext4_rec_len_from_disk(fde->rec_len, blocksize);
- if (unlikely((char *)fde + rlen >= blockend)) {
+ de = ext4_next_entry(dotdot_de, blocksize);
+ if ((char *)de >= (((char *)dot_de) + blocksize)) {
error_msg = "invalid rec_len for '..'";
goto corrupted;
}
-
+ *entry = de;
return true;
corrupted:
@@ -2219,16 +2248,15 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
struct inode *inode, struct buffer_head *bh)
{
struct buffer_head *bh2;
- struct dx_root *root;
struct dx_frame frames[EXT4_HTREE_LEVEL], *frame;
struct dx_entry *entries;
- struct ext4_dir_entry_2 *de, *de2;
+ struct ext4_dir_entry_2 *de, *de2, *dot_de, *dotdot_de;
char *data2, *top;
unsigned len;
int retval;
unsigned blocksize;
ext4_lblk_t block;
- struct fake_dirent *fde;
+ struct dx_root_info *dx_info;
int csum_size = 0;
if (ext4_has_feature_metadata_csum(inode->i_sb))
@@ -2245,17 +2273,20 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
return retval;
}
- root = (struct dx_root *) bh->b_data;
- if (!ext4_check_dx_root(dir, root)) {
+ dot_de = (struct ext4_dir_entry_2 *)bh->b_data;
+ if (ext4_rec_len_from_disk(dot_de->rec_len, blocksize) < EXT4_BASE_DIR_LEN ||
+ ext4_rec_len_from_disk(dot_de->rec_len, blocksize) >= blocksize - EXT4_BASE_DIR_LEN) {
+ brelse(bh);
+ return -EFSCORRUPTED;
+ }
+ dotdot_de = ext4_next_entry(dot_de, blocksize);
+ if (!ext4_check_dx_root(dir, dot_de, dotdot_de, &de)) {
brelse(bh);
return -EFSCORRUPTED;
}
/* The 0th block becomes the root, move the dirents out */
- fde = &root->dotdot;
- de = (struct ext4_dir_entry_2 *)((char *)fde +
- ext4_rec_len_from_disk(fde->rec_len, blocksize));
- len = ((char *) root) + (blocksize - csum_size) - (char *) de;
+ len = ((char *)dot_de) + (blocksize - csum_size) - (char *)de;
/* Allocate new block for the 0th block's dirents */
bh2 = ext4_append(handle, dir, &block);
@@ -2286,24 +2317,32 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
ext4_initialize_dirent_tail(bh2, blocksize);
/* Initialize the root; the dot dirents already exist */
- de = (struct ext4_dir_entry_2 *) (&root->dotdot);
- de->rec_len = ext4_rec_len_to_disk(
- blocksize - ext4_dir_rec_len(2, NULL), blocksize);
- memset (&root->info, 0, sizeof(root->info));
- root->info.info_length = sizeof(root->info);
+ dotdot_de->rec_len =
+ ext4_rec_len_to_disk(blocksize - le16_to_cpu(dot_de->rec_len),
+ blocksize);
+
+ /* initialize hashing info */
+ dx_info = dx_get_dx_info(dir, dot_de);
+ if (IS_ERR(dx_info)) {
+ brelse(bh2);
+ brelse(bh);
+ return PTR_ERR(dx_info);
+ }
+ memset(dx_info, 0, sizeof(*dx_info));
+ dx_info->info_length = sizeof(*dx_info);
if (ext4_hash_in_dirent(dir))
- root->info.hash_version = DX_HASH_SIPHASH;
+ dx_info->hash_version = DX_HASH_SIPHASH;
else
- root->info.hash_version =
+ dx_info->hash_version =
EXT4_SB(dir->i_sb)->s_def_hash_version;
- entries = root->entries;
+ entries = (void *)dx_info + sizeof(*dx_info);
dx_set_block(entries, 1);
dx_set_count(entries, 1);
- dx_set_limit(entries, dx_root_limit(dir, sizeof(root->info)));
+ dx_set_limit(entries, dx_root_limit(dir, sizeof(*dx_info)));
/* Initialize as for dx_probe */
- fname->hinfo.hash_version = root->info.hash_version;
+ fname->hinfo.hash_version = dx_info->hash_version;
if (fname->hinfo.hash_version <= DX_HASH_TEA)
fname->hinfo.hash_version += EXT4_SB(dir->i_sb)->s_hash_unsigned;
fname->hinfo.seed = EXT4_SB(dir->i_sb)->s_hash_seed;
@@ -2346,7 +2385,7 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
*/
if (retval)
ext4_mark_inode_dirty(handle, dir);
- dx_release(frames);
+ dx_release(dir, frames);
brelse(bh2);
return retval;
}
@@ -2613,7 +2652,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname,
if (restart || err)
goto journal_error;
} else {
- struct dx_root *dxroot;
+ struct dx_root_info *info;
memcpy((char *) entries2, (char *) entries,
icount * sizeof(struct dx_entry));
dx_set_limit(entries2, dx_node_limit(dir));
@@ -2621,11 +2660,17 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname,
/* Set up root */
dx_set_count(entries, 1);
dx_set_block(entries + 0, newblock);
- dxroot = (struct dx_root *)frames[0].bh->b_data;
- dxroot->info.indirect_levels += 1;
+ info = dx_get_dx_info(dir, (struct ext4_dir_entry_2 *)
+ frames[0].bh->b_data);
+ if (IS_ERR(info)) {
+ err = PTR_ERR(info);
+ brelse(bh2);
+ goto journal_error;
+ }
+ info->indirect_levels += 1;
dxtrace(printk(KERN_DEBUG
"Creating %d level index...\n",
- dxroot->info.indirect_levels));
+ info->indirect_levels));
err = ext4_handle_dirty_dx_node(handle, dir, frame->bh);
if (err) {
brelse(bh2);
@@ -2649,7 +2694,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname,
ext4_std_error(dir->i_sb, err); /* this is a no-op if err == 0 */
cleanup:
brelse(bh);
- dx_release(frames);
+ dx_release(dir, frames);
/* @restart is true means htree-path has been changed, we need to
* repeat dx_probe() to find out valid htree-path
*/
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v13 05/11] ext4: refactor dx_root to support variable dirent sizes
2026-10-29 12:24 ` [PATCH v13 05/11] ext4: refactor dx_root to support variable dirent sizes Artem Blagodarenko
@ 2026-10-29 12:49 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-10-29 12:49 UTC (permalink / raw)
To: Artem Blagodarenko, linux-ext4; +Cc: tytso
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] ext4: dx_get_dx_info() steps over dot entry using ext4_dir_entry_len()
- [High] ext4: dx_node_limit() change breaks existing encrypted casefold htree dirs
- [High] ext4: make_indexed_dir() corrupts directory state if dx_get_dx_info() fails
--
commit ac1861c16d9ece959b6431b7c2c9637e888e44a0
Author: Artem Blagodarenko <artem.blagodarenko@gmail.com>
ext4: refactor dx_root to support variable dirent sizes
This commit replaces the monolithic struct dx_root definition with a separate
struct dx_root_info and adds dx_get_dx_info() and ext4_dir_entry_len() so
that "." and ".." directory entries in the htree root block can have variable
lengths when carrying dirdata.
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 533340c5dbc0d..59cc01b91c418 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
[ ... ]
> @@ -539,6 +530,31 @@ ext4_next_entry(struct ext4_dir_entry_2 *p, unsigned long blocksize)
> * Future: use high four bits of block for coalesce-on-delete flags
> * Mask them off for now.
> */
> +static struct dx_root_info *dx_get_dx_info(struct inode *dir, void *de_buf)
> +{
> + unsigned int blocksize = dir->i_sb->s_blocksize;
> + void *base = de_buf;
> +
> + /* '.' and '..' never carry the casefold+fscrypt hash, so pass NULL
> + * for dir regardless of the directory's flags */
> + if (ext4_rec_len_from_disk(((struct ext4_dir_entry_2 *)de_buf)->rec_len,
> + blocksize) < EXT4_BASE_DIR_LEN)
> + return ERR_PTR(-EFSCORRUPTED);
> + de_buf += ext4_dir_entry_len(de_buf, blocksize, NULL);
[Severity: High]
Should dx_get_dx_info() advance past the "." entry using ext4_next_entry()
(or dot_de->rec_len) instead of ext4_dir_entry_len()?
Unlike "..", whose rec_len in an indexed root block is set to cover the rest
of the block, "." always stores the actual offset to ".." in dot_de->rec_len.
Other callers locate ".." via dot_de->rec_len, such as make_indexed_dir():
dot_de = (struct ext4_dir_entry_2 *)bh->b_data;
...
dotdot_de = ext4_next_entry(dot_de, blocksize);
and ext4_htree_fill_tree():
de = (struct ext4_dir_entry_2 *) frames[0].bh->b_data;
de = ext4_next_entry(de, dir->i_sb->s_blocksize);
If "." has slack space so that dot_de->rec_len is larger than
ext4_dir_entry_len(dot_de, blocksize, NULL) (for example, dot_de->rec_len is
24 while its minimum entry length is 12), make_indexed_dir() leaves dotdot_de
at offset 24 and preserves dot_de->rec_len:
dotdot_de->rec_len =
ext4_rec_len_to_disk(blocksize - le16_to_cpu(dot_de->rec_len),
blocksize);
/* initialize hashing info */
dx_info = dx_get_dx_info(dir, dot_de);
When dx_get_dx_info() then steps over "." by ext4_dir_entry_len() (12 bytes)
instead of dot_de->rec_len (24 bytes), won't it read the unzeroed slack bytes
after "." as the ".." entry and return a dx_info pointer that overlaps and
overwrites dotdot_de when dx_info and entries[] are initialized?
> +
> + /* dx root info is after dotdot entry */
> + if (de_buf < base || (char *)de_buf - (char *)base +
> + EXT4_BASE_DIR_LEN > blocksize)
> + return ERR_PTR(-EFSCORRUPTED);
> + de_buf += ext4_dir_entry_len(de_buf, blocksize, NULL);
> +
> + if (de_buf < base || (char *)de_buf - (char *)base +
> + sizeof(struct dx_root_info) +
> + sizeof(struct dx_entry) > blocksize)
> + return ERR_PTR(-EFSCORRUPTED);
> +
> + return (struct dx_root_info *)de_buf;
> +}
>
> static inline ext4_lblk_t dx_get_block(struct dx_entry *entry)
> {
> @@ -593,8 +609,10 @@ static inline unsigned dx_root_limit(struct inode *dir, unsigned infosize)
>
> static inline unsigned dx_node_limit(struct inode *dir)
> {
> + /* dx_node fake_dirent is always 8 bytes — it never carries a casefold
> + * hash, so pass NULL to suppress the hash-size term. */
> unsigned int entry_space = dir->i_sb->s_blocksize -
> - ext4_dir_rec_len(0, dir);
> + ext4_dir_rec_len(0, NULL);
[Severity: High]
Won't changing ext4_dir_rec_len(0, dir) to ext4_dir_rec_len(0, NULL) in
dx_node_limit() break existing multi-level encrypted and casefolded htree
directories?
In fs/ext4/ext4.h:
static inline unsigned int ext4_dir_rec_len(__u8 name_len,
const struct inode *dir)
{
int rec_len = (name_len + 8 + EXT4_DIR_ROUND);
if (dir && ext4_hash_in_dirent(dir))
rec_len += sizeof(struct ext4_dir_entry_hash);
return (rec_len & ~EXT4_DIR_ROUND);
}
For directories where ext4_hash_in_dirent(dir) is true, ext4_dir_rec_len(0,
dir) returns 16, whereas ext4_dir_rec_len(0, NULL) returns 8. Passing NULL
increases dx_node_limit(dir) by 1 entry (for example, from 509 to 510 on 4KB
blocks with metadata_csum enabled).
Existing multi-level encrypted + casefolded directories created since commit
471fbbea7ff7 ("ext4: handle casefolding with encryption") have the smaller
limit recorded on disk in each internal dx_node block. When dx_probe() reads
an internal node, it enforces an exact match:
entries = ((struct dx_node *) frame->bh->b_data)->entries;
if (dx_get_limit(entries) != dx_node_limit(dir)) {
ext4_warning_inode(dir,
"dx entry: limit %u != node limit %u",
dx_get_limit(entries), dx_node_limit(dir));
goto fail;
}
Won't dx_probe() reject all existing multi-level encrypted + casefolded
directories as corrupt with ERR_BAD_DX_DIR, causing __ext4_add_entry() to
trigger EXT4_ERROR_INODE() when metadata_csum is enabled?
[ ... ]
> @@ -2286,24 +2317,32 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
> ext4_initialize_dirent_tail(bh2, blocksize);
>
> /* Initialize the root; the dot dirents already exist */
> - de = (struct ext4_dir_entry_2 *) (&root->dotdot);
> - de->rec_len = ext4_rec_len_to_disk(
> - blocksize - ext4_dir_rec_len(2, NULL), blocksize);
> - memset (&root->info, 0, sizeof(root->info));
> - root->info.info_length = sizeof(root->info);
> + dotdot_de->rec_len =
> + ext4_rec_len_to_disk(blocksize - le16_to_cpu(dot_de->rec_len),
> + blocksize);
> +
> + /* initialize hashing info */
> + dx_info = dx_get_dx_info(dir, dot_de);
> + if (IS_ERR(dx_info)) {
> + brelse(bh2);
> + brelse(bh);
> + return PTR_ERR(dx_info);
> + }
[Severity: High]
If dx_get_dx_info() fails here in make_indexed_dir(), is it safe to return
early after the directory block and inode have already been modified?
By the time dx_get_dx_info() is called, make_indexed_dir() has already
appended bh2 (extending dir->i_size and marking the inode dirty in the
running handle), set EXT4_INODE_INDEX on dir, wiped the original entries in
block 0 (bh) with memset(de, 0, len), and updated dotdot_de->rec_len to span
the rest of block 0:
bh2 = ext4_append(handle, dir, &block);
if (IS_ERR(bh2)) {
brelse(bh);
return PTR_ERR(bh2);
}
ext4_set_inode_flag(dir, EXT4_INODE_INDEX);
data2 = bh2->b_data;
memcpy(data2, de, len);
memset(de, 0, len); /* wipe old data */
Returning PTR_ERR(dx_info) at this point leaves dir with EXT4_INODE_INDEX set
and an extended i_size, while block 0 has its entries wiped and no valid
dx_root_info initialized. Could dx_get_dx_info(dir, dot_de) be validated
before ext4_append() and memset() modify the directory?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029122413.230409-1-ablagodarenko@thelustrecollective.com?part=5
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v13 06/11] ext4: add ext4_dir_entry_len()
2026-10-29 12:23 [PATCH v13 00/11] Data in direntry (dirdata) feature Artem Blagodarenko
` (4 preceding siblings ...)
2026-10-29 12:24 ` [PATCH v13 05/11] ext4: refactor dx_root to support variable dirent sizes Artem Blagodarenko
@ 2026-10-29 12:24 ` Artem Blagodarenko
2026-10-29 12:50 ` sashiko-bot
2026-10-29 12:24 ` [PATCH v13 07/11] ext4: rename ext4_dir_rec_len() and clarify dirdata usage Artem Blagodarenko
` (4 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Artem Blagodarenko @ 2026-10-29 12:24 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, jack, adilger.kernel, ablagodarenko, Artem Blagodarenko,
Andreas Dilger
From: Artem Blagodarenko <artem.blagodarenko@gmail.com>
ext4_dir_entry_len() (added in the previous patch, where
dx_get_dx_info() first needs it) computes the required rec_len for
a directory entry, taking into account dirdata and casefold+fscrypt
hash space.
Update dx_root_limit() to use ext4_dir_entry_len() instead of
open-coded ext4_dir_rec_len() for '.' and '..' entries.
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
---
fs/ext4/ext4.h | 18 ++++++++++++++----
fs/ext4/namei.c | 25 +++++++++++++++++--------
2 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index edb95cbc4439..2d6d3be4ef7d 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2655,7 +2655,15 @@ static inline unsigned int ext4_dir_rec_len(__u8 name_len,
{
int rec_len = (name_len + 8 + EXT4_DIR_ROUND);
- if (dir && ext4_hash_in_dirent(dir))
+ /*
+ * Without dirdata, the casefold+fscrypt hash lives at a fixed position
+ * after the filename and must be reserved explicitly. With dirdata the
+ * hash is stored as a CFHASH extension and is already counted in the
+ * name_len argument (via ext4_dirent_get_data_len), so adding it again
+ * would double-count.
+ */
+ if (dir && ext4_hash_in_dirent(dir) &&
+ !ext4_has_feature_dirdata(dir->i_sb))
rec_len += sizeof(struct ext4_dir_entry_hash);
return (rec_len & ~EXT4_DIR_ROUND);
}
@@ -3079,12 +3087,14 @@ static const unsigned char ext4_filetype_table[] = {
DT_UNKNOWN, DT_REG, DT_DIR, DT_CHR, DT_BLK, DT_FIFO, DT_SOCK, DT_LNK
};
-static inline unsigned char get_dtype(struct super_block *sb, int filetype)
+static inline unsigned char get_dtype(struct super_block *sb, int filetype)
{
- if (!ext4_has_feature_filetype(sb) || filetype >= EXT4_FT_MAX)
+ unsigned char fl_index = filetype & EXT4_FT_MASK;
+
+ if (!ext4_has_feature_filetype(sb) || fl_index >= EXT4_FT_MAX)
return DT_UNKNOWN;
- return ext4_filetype_table[filetype];
+ return ext4_filetype_table[fl_index];
}
extern int ext4_check_all_de(struct inode *dir, struct buffer_head *bh,
void *buf, int buf_size);
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 59cc01b91c41..b3acab0689c9 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -596,11 +596,17 @@ static inline void dx_set_limit(struct dx_entry *entries, unsigned value)
((struct dx_countlimit *) entries)->limit = cpu_to_le16(value);
}
-static inline unsigned dx_root_limit(struct inode *dir, unsigned infosize)
+static inline unsigned dx_root_limit(struct inode *dir,
+ struct ext4_dir_entry_2 *dot_de)
{
- unsigned int entry_space = dir->i_sb->s_blocksize -
- ext4_dir_rec_len(1, NULL) -
- ext4_dir_rec_len(2, NULL) - infosize;
+ struct dx_root_info *info;
+ unsigned int entry_space;
+
+ info = dx_get_dx_info(dir, dot_de);
+ if (IS_ERR(info))
+ return 0;
+ entry_space = dir->i_sb->s_blocksize - ((char *)info - (char *)dot_de) -
+ info->info_length;
if (ext4_has_feature_metadata_csum(dir->i_sb))
entry_space -= sizeof(struct dx_tail);
@@ -885,10 +891,13 @@ dx_probe(struct ext4_filename *fname, struct inode *dir,
entries = (struct dx_entry *)(((char *)info) + info->info_length);
- if (dx_get_limit(entries) != dx_root_limit(dir, info->info_length)) {
+ if (dx_get_limit(entries) !=
+ dx_root_limit(dir, (struct ext4_dir_entry_2 *)frame->bh->b_data)) {
ext4_warning_inode(dir, "dx entry: limit %u != root limit %u",
dx_get_limit(entries),
- dx_root_limit(dir, info->info_length));
+ dx_root_limit(dir,
+ (struct ext4_dir_entry_2 *)frame->bh->b_data
+ ));
goto fail;
}
@@ -2336,10 +2345,10 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
dx_info->hash_version =
EXT4_SB(dir->i_sb)->s_def_hash_version;
- entries = (void *)dx_info + sizeof(*dx_info);
+ entries = (void *)dx_info + dx_info->info_length;
dx_set_block(entries, 1);
dx_set_count(entries, 1);
- dx_set_limit(entries, dx_root_limit(dir, sizeof(*dx_info)));
+ dx_set_limit(entries, dx_root_limit(dir, dot_de));
/* Initialize as for dx_probe */
fname->hinfo.hash_version = dx_info->hash_version;
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v13 06/11] ext4: add ext4_dir_entry_len()
2026-10-29 12:24 ` [PATCH v13 06/11] ext4: add ext4_dir_entry_len() Artem Blagodarenko
@ 2026-10-29 12:50 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-10-29 12:50 UTC (permalink / raw)
To: Artem Blagodarenko, linux-ext4; +Cc: tytso
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] ext4: missing hash size in ext4_dir_rec_len() when dirdata is enabled
- [High] ext4: inconsistent dot_de offset in make_indexed_dir() and dx_get_dx_info()
- [High] ext4: insufficient space check in dx_get_dx_info() and dx_root_limit()
--
commit cde4b627ee0c8478f3506f3aca76ba7f76852f15
Author: Artem Blagodarenko <artem.blagodarenko@gmail.com>
ext4: add ext4_dir_entry_len()
This commit updates ext4_dir_rec_len() to skip adding sizeof(struct
ext4_dir_entry_hash) when the dirdata feature is enabled, masks the filetype
in get_dtype(), and updates dx_root_limit() and its callers to compute the
root htree limit via dx_get_dx_info().
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index edb95cbc4439e..2d6d3be4ef7df 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2655,7 +2655,15 @@ static inline unsigned int ext4_dir_rec_len(__u8 name_len,
> {
> int rec_len = (name_len + 8 + EXT4_DIR_ROUND);
>
> - if (dir && ext4_hash_in_dirent(dir))
> + /*
> + * Without dirdata, the casefold+fscrypt hash lives at a fixed position
> + * after the filename and must be reserved explicitly. With dirdata the
> + * hash is stored as a CFHASH extension and is already counted in the
> + * name_len argument (via ext4_dirent_get_data_len), so adding it again
> + * would double-count.
> + */
> + if (dir && ext4_hash_in_dirent(dir) &&
> + !ext4_has_feature_dirdata(dir->i_sb))
> rec_len += sizeof(struct ext4_dir_entry_hash);
> return (rec_len & ~EXT4_DIR_ROUND);
> }
[Severity: High]
Can skipping sizeof(struct ext4_dir_entry_hash) whenever
ext4_has_feature_dirdata(dir->i_sb) is set cause out-of-bounds reads and
directory entry corruption on encrypted casefolded directories?
At this commit, callers such as ext4_find_dest_de(), ext4_insert_dentry(),
dx_move_dirents(), dx_pack_dirents(), and __ext4_check_dir_entry() still pass
de->name_len or fname_len(fname) directly to ext4_dir_rec_len() without
including ext4_dirent_get_data_len(). Even after later patches in the series
("ext4: rename ext4_dir_rec_len() and clarify dirdata usage", "ext4: dirdata
feature", and "ext4: add dirdata set/get helpers") convert some callers to
ext4_dir_entry_len(), several paths remain broken through the end of the
series:
1. In fs/ext4/namei.c:ext4_search_dir(), the bounds check before ext4_match()
still calls ext4_dir_rec_len() (renamed to ext4_dirent_rec_len()) directly on
de->name_len:
if (de->name + de->name_len <= dlimit &&
(!ext4_hash_in_dirent(dir) ||
(char *)de + ext4_dir_rec_len(de->name_len, dir) <= dlimit) &&
ext4_match(dir, fname, de)) {
When dirdata and casefold+fscrypt are enabled on dir, ext4_dir_rec_len()
omits the 8-byte hash size. If an entry at the end of search_buf does not
have EXT4_DIRENT_CFHASH set in de->file_type, this bounds check passes when
only de->name_len fits before dlimit, and ext4_match() then falls back to
EXT4_DIRENT_HASH(de) and EXT4_DIRENT_MINOR_HASH(de), reading 8 bytes past
dlimit.
2. In fs/ext4/ext4.h:ext4_dir_entry_len():
static inline unsigned int ext4_dir_entry_len(struct ext4_dir_entry_2 *de,
unsigned int blocksize,
const struct inode *dir)
{
unsigned int rec_len = ext4_rec_len_from_disk(de->rec_len, blocksize);
unsigned int dirdata = ext4_dirent_get_data_len(de, rec_len);
return ext4_dir_rec_len(de->name_len + dirdata, dir);
}
If an entry in a casefolded+encrypted directory on a dirdata filesystem does
not have EXT4_DIRENT_CFHASH set in de->file_type (either a legacy un-migrated
entry or a corrupted on-disk entry), ext4_dirent_get_data_len() returns 0 and
ext4_dir_rec_len() also skips adding sizeof(struct ext4_dir_entry_hash).
Consequently, __ext4_check_dir_entry() accepts a trailing entry whose rec_len
only covers de->name_len, and the !EXT4_DIRENT_CFHASH fallback paths in
ext4_readdir(), htree_dirblock_to_tree(), dx_make_map(), and
ext4_inlinedir_to_tree() read EXT4_DIRENT_HASH(de) and
EXT4_DIRENT_MINOR_HASH(de) 8 bytes past the end of the block buffer or
inline directory slab buffer. For un-migrated entries, ext4_dir_entry_len()
also under-reports the entry length by 8 bytes, causing ext4_find_dest_de(),
ext4_insert_dentry(), dx_move_dirents(), and dx_pack_dirents() to overwrite
the legacy hash.
3. Callers passing literal 0 or 1 to compute minimum dirent size
(__ext4_check_dir_entry(), ext4_readdir(), and htree_dirblock_to_tree()) get
8 or 12 bytes instead of 16 or 20 bytes when dirdata is enabled on a
casefolded+encrypted directory.
4. Conversely, if !ext4_has_feature_dirdata(dir->i_sb) and
ext4_hash_in_dirent(dir) is true, an on-disk entry with EXT4_DIRENT_CFHASH
set in de->file_type has its extension counted by ext4_dirent_get_data_len()
and sizeof(struct ext4_dir_entry_hash) added a second time by
ext4_dir_rec_len().
Should the hash size inclusion depend on whether the hash is already accounted
for by EXT4_DIRENT_CFHASH (and whether the caller actually queried dirdata),
rather than solely on !ext4_has_feature_dirdata(dir->i_sb)?
[ ... ]
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 59cc01b91c418..b3acab0689c93 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -596,11 +596,17 @@ static inline void dx_set_limit(struct dx_entry *entries, unsigned value)
> ((struct dx_countlimit *) entries)->limit = cpu_to_le16(value);
> }
>
> -static inline unsigned dx_root_limit(struct inode *dir, unsigned infosize)
> +static inline unsigned dx_root_limit(struct inode *dir,
> + struct ext4_dir_entry_2 *dot_de)
> {
> - unsigned int entry_space = dir->i_sb->s_blocksize -
> - ext4_dir_rec_len(1, NULL) -
> - ext4_dir_rec_len(2, NULL) - infosize;
> + struct dx_root_info *info;
> + unsigned int entry_space;
> +
> + info = dx_get_dx_info(dir, dot_de);
> + if (IS_ERR(info))
> + return 0;
> + entry_space = dir->i_sb->s_blocksize - ((char *)info - (char *)dot_de) -
> + info->info_length;
>
> if (ext4_has_feature_metadata_csum(dir->i_sb))
> entry_space -= sizeof(struct dx_tail);
> @@ -885,10 +891,13 @@ dx_probe(struct ext4_filename *fname, struct inode *dir,
>
> entries = (struct dx_entry *)(((char *)info) + info->info_length);
>
> - if (dx_get_limit(entries) != dx_root_limit(dir, info->info_length)) {
> + if (dx_get_limit(entries) !=
> + dx_root_limit(dir, (struct ext4_dir_entry_2 *)frame->bh->b_data)) {
> ext4_warning_inode(dir, "dx entry: limit %u != root limit %u",
> dx_get_limit(entries),
> - dx_root_limit(dir, info->info_length));
> + dx_root_limit(dir,
> + (struct ext4_dir_entry_2 *)frame->bh->b_data
> + ));
> goto fail;
> }
>
> @@ -2336,10 +2345,10 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
> dx_info->hash_version =
> EXT4_SB(dir->i_sb)->s_def_hash_version;
>
> - entries = (void *)dx_info + sizeof(*dx_info);
> + entries = (void *)dx_info + dx_info->info_length;
> dx_set_block(entries, 1);
> dx_set_count(entries, 1);
> - dx_set_limit(entries, dx_root_limit(dir, sizeof(*dx_info)));
> + dx_set_limit(entries, dx_root_limit(dir, dot_de));
[Severity: High]
What happens in make_indexed_dir() when dot_de->rec_len does not equal
ext4_dir_entry_len(dot_de, blocksize, NULL)?
In fs/ext4/namei.c:make_indexed_dir(), dotdot_de is located using
dot_de->rec_len via ext4_next_entry(dot_de, blocksize), and left in place
while setting its new rec_len:
dotdot_de = ext4_next_entry(dot_de, blocksize);
if (!ext4_check_dx_root(dir, dot_de, dotdot_de, &de)) {
brelse(bh);
return -EFSCORRUPTED;
}
...
dotdot_de->rec_len =
ext4_rec_len_to_disk(blocksize - le16_to_cpu(dot_de->rec_len),
blocksize);
/* initialize hashing info */
dx_info = dx_get_dx_info(dir, dot_de);
However, dx_get_dx_info() (which is now also called by dx_root_limit()) steps
over dot_de using ext4_dir_entry_len(de_buf, blocksize, NULL) instead of
dot_de->rec_len:
de_buf += ext4_dir_entry_len(de_buf, blocksize, NULL);
If dot_de->rec_len has padding (e.g. 16 or 24 bytes when '.' has no dirdata)
or is 8 bytes, ext4_check_dx_root() still succeeds because it does not verify
that dot_de->rec_len matches ext4_dir_entry_len(dot_de, blocksize, NULL).
dx_get_dx_info() then interprets bytes at dot_de + ext4_dir_entry_len(dot_de)
(inside dot_de's unzeroed slack space or misaligned with dotdot_de) as the
'..' entry, placing dx_info and entries[] at an offset that overwrites
dotdot_de and disagrees with get_dx_countlimit() (which steps over dot_de
using dirent->rec_len in ext4_dx_csum_set()).
In addition, does dx_get_dx_info() or make_indexed_dir() ensure enough space
remains in the root block for struct dx_tail and the second dx_entry created
by do_split()?
In dx_get_dx_info(), the bounds check only requires sizeof(struct
dx_root_info) + sizeof(struct dx_entry) (16 bytes) to remain in the block:
if (de_buf < base || (char *)de_buf - (char *)base +
sizeof(struct dx_root_info) +
sizeof(struct dx_entry) > blocksize)
return ERR_PTR(-EFSCORRUPTED);
When '.' and '..' carry large dirdata extensions (particularly once the next
commit widens ext4_dirent_rec_len()'s length parameter) such that only 16 to
23 bytes remain in the block after '..', dx_get_dx_info() succeeds, but:
1. In dx_root_limit(), subtracting sizeof(struct dx_tail) (8 bytes) when
metadata_csum is enabled leaves entry_space < sizeof(struct dx_entry), so
dx_root_limit() returns 0 (or if info->info_length is corrupted and large on
disk, subtracting info->info_length and sizeof(struct dx_tail) can underflow
unsigned int entry_space).
2. make_indexed_dir() does not check that dx_root_limit(dir, dot_de) >= 2.
When (char *)dx_info - (char *)dot_de == blocksize - 16, entries[0] is
written with count = 1 and limit = 0 (triggering EXT4_ERROR_INODE() in
ext4_handle_dirty_dx_node() -> ext4_dx_csum_set()), and then do_split() ->
dx_insert_block() writes entries[1] at bh->b_data + blocksize, 8 bytes past
the end of the buffer.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029122413.230409-1-ablagodarenko@thelustrecollective.com?part=6
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v13 07/11] ext4: rename ext4_dir_rec_len() and clarify dirdata usage
2026-10-29 12:23 [PATCH v13 00/11] Data in direntry (dirdata) feature Artem Blagodarenko
` (5 preceding siblings ...)
2026-10-29 12:24 ` [PATCH v13 06/11] ext4: add ext4_dir_entry_len() Artem Blagodarenko
@ 2026-10-29 12:24 ` Artem Blagodarenko
2026-10-29 12:50 ` sashiko-bot
2026-10-29 12:24 ` [PATCH v13 08/11] ext4: dirdata feature Artem Blagodarenko
` (3 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Artem Blagodarenko @ 2026-10-29 12:24 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, jack, adilger.kernel, ablagodarenko, Artem Blagodarenko,
Andreas Dilger
From: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Rename ext4_dir_rec_len() to ext4_dirent_rec_len() to better
reflect that it computes the record length for a directory
entry based on the provided name length.
Update the comment to clarify handling of dirdata-enabled
directories and document the use of ext4_dir_entry_len()
when dirdata is present.
No functional changes.
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
---
fs/ext4/dir.c | 15 +++++++--------
fs/ext4/ext4.h | 14 ++++++++++----
fs/ext4/inline.c | 21 ++++++++++++---------
fs/ext4/namei.c | 33 +++++++++++++++++----------------
4 files changed, 46 insertions(+), 37 deletions(-)
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 8d7b81e6948e..38911ae2bcfc 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -89,16 +89,15 @@ int __ext4_check_dir_entry(const char *function, unsigned int line,
bool fake = is_fake_dir_entry(de);
bool has_csum = ext4_has_feature_metadata_csum(dir->i_sb);
- if (unlikely(rlen < ext4_dir_rec_len(1, fake ? NULL : dir)))
+ if (unlikely(rlen < ext4_dirent_rec_len(1, fake ? NULL : dir)))
error_msg = "rec_len is smaller than minimal";
else if (unlikely(rlen % 4 != 0))
error_msg = "rec_len % 4 != 0";
- else if (unlikely(rlen < ext4_dir_rec_len(de->name_len,
- fake ? NULL : dir)))
- error_msg = "rec_len is too small for name_len";
else if (unlikely(next_offset > size))
error_msg = "directory entry overrun";
- else if (unlikely(next_offset > size - ext4_dir_rec_len(1,
+ else if (unlikely(rlen < ext4_dir_entry_len(de, size, fake ? NULL : dir)))
+ error_msg = "rec_len is too small for name_len";
+ else if (unlikely(next_offset > size - ext4_dirent_rec_len(1,
has_csum ? NULL : dir) &&
next_offset != size))
error_msg = "directory entry too close to block end";
@@ -238,7 +237,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
if (!inode_eq_iversion(inode, info->cookie)) {
for (i = 0;
i <= sb->s_blocksize -
- ext4_dir_rec_len(1, has_csum ? NULL : inode) &&
+ ext4_dirent_rec_len(1, has_csum ? NULL : inode) &&
i < offset;) {
de = (struct ext4_dir_entry_2 *)
(bh->b_data + i);
@@ -249,7 +248,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
* failure will be detected in the
* dirent test below. */
if (ext4_rec_len_from_disk(de->rec_len,
- sb->s_blocksize) < ext4_dir_rec_len(1,
+ sb->s_blocksize) < ext4_dirent_rec_len(1,
inode))
break;
i += ext4_rec_len_from_disk(de->rec_len,
@@ -263,7 +262,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
if (unlikely(offset < sb->s_blocksize &&
offset > sb->s_blocksize -
- ext4_dir_rec_len(1, has_csum ? NULL : inode))) {
+ ext4_dirent_rec_len(1, has_csum ? NULL : inode))) {
EXT4_ERROR_FILE(file, bh->b_blocknr,
"bad entry in directory: %s - offset=%u, size=%lu",
"directory entry too close to block end",
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 2d6d3be4ef7d..839274c4d7fc 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2649,11 +2649,16 @@ struct ext4_dirent_hash {
* casefolded and encrypted need to store the hash as well, so we add room for
* ext4_extended_dir_entry_2. For all entries related to '.' or '..' you should
* pass NULL for dir, as those entries do not use the extra fields.
+ *
+ * For directories with the dirdata feature, extra data may follow the filename.
+ * Use ext4_dir_entry_len() to compute the length of a directory entry
+ * including any dirdata, or ext4_dirent_rec_len() directly when the total
+ * name_len (including dirdata length) is already known.
*/
-static inline unsigned int ext4_dir_rec_len(__u8 name_len,
+static inline unsigned int ext4_dirent_rec_len(unsigned int name_len,
const struct inode *dir)
{
- int rec_len = (name_len + 8 + EXT4_DIR_ROUND);
+ unsigned int rec_len = (name_len + 8 + EXT4_DIR_ROUND);
/*
* Without dirdata, the casefold+fscrypt hash lives at a fixed position
@@ -3069,7 +3074,8 @@ extern void ext4_htree_free_dir_info(struct dir_private_info *p);
extern int ext4_find_dest_de(struct inode *dir, struct buffer_head *bh,
void *buf, int buf_size,
struct ext4_filename *fname,
- struct ext4_dir_entry_2 **dest_de);
+ struct ext4_dir_entry_2 **dest_de,
+ int dlen);
void ext4_insert_dentry(struct inode *dir, struct inode *inode,
struct ext4_dir_entry_2 *de,
int buf_size,
@@ -4266,7 +4272,7 @@ static inline unsigned int ext4_dir_entry_len(struct ext4_dir_entry_2 *de,
unsigned int rec_len = ext4_rec_len_from_disk(de->rec_len, blocksize);
unsigned int dirdata = ext4_dirent_get_data_len(de, rec_len);
- return ext4_dir_rec_len(de->name_len + dirdata, dir);
+ return ext4_dirent_rec_len(de->name_len + dirdata, dir);
}
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index ceee69a66482..bdf6f45a3de4 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -985,8 +985,11 @@ static int ext4_add_dirent_to_inline(handle_t *handle,
int err;
struct ext4_dir_entry_2 *de;
+ /* 'dlen' reserves space for dirdata extensions (LUFID, CFHASH) in the
+ * slot. The dirdata types are defined in the next patch; until then
+ * dlen stays 0 and ext4_find_dest_de() ignores dirdata overhead. */
err = ext4_find_dest_de(dir, iloc->bh, inline_start,
- inline_size, fname, &de);
+ inline_size, fname, &de, 0);
if (err)
return err;
@@ -1064,7 +1067,7 @@ static int ext4_update_inline_dir(handle_t *handle, struct inode *dir,
int old_size = EXT4_I(dir)->i_inline_size - EXT4_MIN_INLINE_DATA_SIZE;
int new_size = get_max_inline_xattr_value_size(dir, iloc);
- if (new_size - old_size <= ext4_dir_rec_len(1, NULL))
+ if (new_size - old_size <= ext4_dirent_rec_len(1, NULL))
return -ENOSPC;
ret = ext4_update_inline_data(handle, dir,
@@ -1318,7 +1321,7 @@ int ext4_inlinedir_to_tree(struct file *dir_file,
fake.name_len = 1;
memcpy(fake.name, ".", 2);
fake.rec_len = ext4_rec_len_to_disk(
- ext4_dir_rec_len(fake.name_len, NULL),
+ ext4_dirent_rec_len(fake.name_len, NULL),
inline_size);
ext4_set_de_type(inode->i_sb, &fake, S_IFDIR);
de = &fake;
@@ -1328,7 +1331,7 @@ int ext4_inlinedir_to_tree(struct file *dir_file,
fake.name_len = 2;
memcpy(fake.name, "..", 3);
fake.rec_len = ext4_rec_len_to_disk(
- ext4_dir_rec_len(fake.name_len, NULL),
+ ext4_dirent_rec_len(fake.name_len, NULL),
inline_size);
ext4_set_de_type(inode->i_sb, &fake, S_IFDIR);
de = &fake;
@@ -1436,8 +1439,8 @@ int ext4_read_inline_dir(struct file *file,
* So we will use extra_offset and extra_size to indicate them
* during the inline dir iteration.
*/
- dotdot_offset = ext4_dir_rec_len(1, NULL);
- dotdot_size = dotdot_offset + ext4_dir_rec_len(2, NULL);
+ dotdot_offset = ext4_dirent_rec_len(1, NULL);
+ dotdot_size = dotdot_offset + ext4_dirent_rec_len(2, NULL);
extra_offset = dotdot_size - EXT4_INLINE_DOTDOT_SIZE;
extra_size = extra_offset + inline_size;
@@ -1463,7 +1466,7 @@ int ext4_read_inline_dir(struct file *file,
/* for other entry, the real offset in
* the buf has to be tuned accordingly.
*/
- if (i + ext4_dir_rec_len(1, NULL) > extra_size)
+ if (i + ext4_dirent_rec_len(1, NULL) > extra_size)
break;
de = (struct ext4_dir_entry_2 *)
(dir_buf + i - extra_offset);
@@ -1474,7 +1477,7 @@ int ext4_read_inline_dir(struct file *file,
* failure will be detected in the
* dirent test below. */
if (ext4_rec_len_from_disk(de->rec_len, extra_size)
- < ext4_dir_rec_len(1, NULL))
+ < ext4_dirent_rec_len(1, NULL))
break;
i += ext4_rec_len_from_disk(de->rec_len,
extra_size);
@@ -1504,7 +1507,7 @@ int ext4_read_inline_dir(struct file *file,
* kmalloc(inline_size) buffer. Make sure its header fits before
* ext4_check_dir_entry() dereferences de->rec_len.
*/
- if (ctx->pos + ext4_dir_rec_len(1, NULL) > extra_size)
+ if (ctx->pos + ext4_dirent_rec_len(1, NULL) > extra_size)
goto out;
de = (struct ext4_dir_entry_2 *)
(dir_buf + ctx->pos - extra_offset);
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index b3acab0689c9..e54608950fb3 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -618,7 +618,7 @@ static inline unsigned dx_node_limit(struct inode *dir)
/* dx_node fake_dirent is always 8 bytes — it never carries a casefold
* hash, so pass NULL to suppress the hash-size term. */
unsigned int entry_space = dir->i_sb->s_blocksize -
- ext4_dir_rec_len(0, NULL);
+ ext4_dirent_rec_len(0, NULL);
if (ext4_has_feature_metadata_csum(dir->i_sb))
entry_space -= sizeof(struct dx_tail);
@@ -728,7 +728,7 @@ static struct stats dx_show_leaf(struct inode *dir,
(unsigned) ((char *) de - base));
#endif
}
- space += ext4_dir_rec_len(de->name_len, dir);
+ space += ext4_dirent_rec_len(de->name_len, dir);
names++;
}
de = ext4_next_entry(de, size);
@@ -1106,7 +1106,7 @@ static int htree_dirblock_to_tree(struct file *dir_file,
/* csum entries are not larger in the casefolded encrypted case */
top = (struct ext4_dir_entry_2 *) ((char *) de +
dir->i_sb->s_blocksize -
- ext4_dir_rec_len(0,
+ ext4_dirent_rec_len(0,
csum ? NULL : dir));
/* Check if the directory is encrypted */
if (IS_ENCRYPTED(dir)) {
@@ -1518,7 +1518,7 @@ int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size,
/* do minimal checking `by hand' */
if (de->name + de->name_len <= dlimit &&
(!ext4_hash_in_dirent(dir) ||
- (char *)de + ext4_dir_rec_len(de->name_len, dir) <= dlimit) &&
+ (char *)de + ext4_dirent_rec_len(de->name_len, dir) <= dlimit) &&
ext4_match(dir, fname, de)) {
/* found a match - just to be sure, do
* a full check */
@@ -1902,7 +1902,7 @@ dx_move_dirents(struct inode *dir, char *from, char *to,
while (count--) {
struct ext4_dir_entry_2 *de = (struct ext4_dir_entry_2 *)
(from + (map->offs<<2));
- rec_len = ext4_dir_rec_len(de->name_len, dir);
+ rec_len = ext4_dir_entry_len(de, blocksize, dir);
memcpy (to, de, rec_len);
((struct ext4_dir_entry_2 *) to)->rec_len =
@@ -1935,7 +1935,7 @@ static struct ext4_dir_entry_2 *dx_pack_dirents(struct inode *dir, char *base,
while ((char*)de < base + blocksize) {
next = ext4_next_entry(de, blocksize);
if (de->inode && de->name_len) {
- rec_len = ext4_dir_rec_len(de->name_len, dir);
+ rec_len = ext4_dir_entry_len(de, blocksize, dir);
if (de > to)
memmove(to, de, rec_len);
to->rec_len = ext4_rec_len_to_disk(rec_len, blocksize);
@@ -2087,10 +2087,11 @@ static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
int ext4_find_dest_de(struct inode *dir, struct buffer_head *bh,
void *buf, int buf_size,
struct ext4_filename *fname,
- struct ext4_dir_entry_2 **dest_de)
+ struct ext4_dir_entry_2 **dest_de,
+ int dlen)
{
struct ext4_dir_entry_2 *de;
- unsigned short reclen = ext4_dir_rec_len(fname_len(fname), dir);
+ unsigned short reclen = ext4_dirent_rec_len(fname_len(fname) + dlen, dir);
int nlen, rlen;
unsigned int offset = 0;
char *top;
@@ -2103,7 +2104,7 @@ int ext4_find_dest_de(struct inode *dir, struct buffer_head *bh,
return -EFSCORRUPTED;
if (ext4_match(dir, fname, de))
return -EEXIST;
- nlen = ext4_dir_rec_len(de->name_len, dir);
+ nlen = ext4_dir_entry_len(de, buf_size, dir);
rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
if ((de->inode ? rlen - nlen : rlen) >= reclen)
break;
@@ -2126,7 +2127,7 @@ void ext4_insert_dentry(struct inode *dir,
int nlen, rlen;
- nlen = ext4_dir_rec_len(de->name_len, dir);
+ nlen = ext4_dir_entry_len(de, buf_size, dir);
rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
if (de->inode) {
struct ext4_dir_entry_2 *de1 =
@@ -2171,7 +2172,7 @@ static int add_dirent_to_buf(handle_t *handle, struct ext4_filename *fname,
if (!de) {
err = ext4_find_dest_de(dir, bh, bh->b_data,
- blocksize - csum_size, fname, &de);
+ blocksize - csum_size, fname, &de, 0);
if (err)
return err;
}
@@ -2995,7 +2996,7 @@ int ext4_init_dirblock(handle_t *handle, struct inode *inode,
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),
+ de->rec_len = ext4_rec_len_to_disk(ext4_dirent_rec_len(de->name_len, NULL),
blocksize);
memcpy(de->name, ".", 2);
ext4_set_de_type(inode->i_sb, de, S_IFDIR);
@@ -3007,7 +3008,7 @@ int ext4_init_dirblock(handle_t *handle, struct inode *inode,
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),
+ ext4_dirent_rec_len(de->name_len, NULL),
blocksize);
de = ext4_next_entry(de, blocksize);
header_size = (char *)de - bh->b_data;
@@ -3016,7 +3017,7 @@ int ext4_init_dirblock(handle_t *handle, struct inode *inode,
blocksize - csum_size);
} else {
de->rec_len = ext4_rec_len_to_disk(blocksize -
- (csum_size + ext4_dir_rec_len(1, NULL)),
+ (csum_size + ext4_dirent_rec_len(1, NULL)),
blocksize);
}
@@ -3139,8 +3140,8 @@ bool ext4_empty_dir(struct inode *inode)
}
sb = inode->i_sb;
- if (inode->i_size < ext4_dir_rec_len(1, NULL) +
- ext4_dir_rec_len(2, NULL)) {
+ if (inode->i_size < ext4_dirent_rec_len(1, NULL) +
+ ext4_dirent_rec_len(2, NULL)) {
EXT4_ERROR_INODE(inode, "invalid size");
return false;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v13 07/11] ext4: rename ext4_dir_rec_len() and clarify dirdata usage
2026-10-29 12:24 ` [PATCH v13 07/11] ext4: rename ext4_dir_rec_len() and clarify dirdata usage Artem Blagodarenko
@ 2026-10-29 12:50 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-10-29 12:50 UTC (permalink / raw)
To: Artem Blagodarenko, linux-ext4; +Cc: tytso
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] ext4: mutable ext4_dir_entry_len() causes OOB write in ext4_insert_dentry()
--
commit da71cae2acaae8de2bca9ec5b4ce285d12415d14
Author: Artem Blagodarenko <artem.blagodarenko@gmail.com>
ext4: rename ext4_dir_rec_len() and clarify dirdata usage
This commit renames ext4_dir_rec_len() to ext4_dirent_rec_len(), widens its
name_len parameter from __u8 to unsigned int, and updates several callers to
use ext4_dir_entry_len() or pass an explicit dirdata length to
ext4_find_dest_de().
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index 8d7b81e6948e7..38911ae2bcfc0 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -89,16 +89,15 @@ int __ext4_check_dir_entry(const char *function, unsigned int line,
> bool fake = is_fake_dir_entry(de);
> bool has_csum = ext4_has_feature_metadata_csum(dir->i_sb);
>
> - if (unlikely(rlen < ext4_dir_rec_len(1, fake ? NULL : dir)))
> + if (unlikely(rlen < ext4_dirent_rec_len(1, fake ? NULL : dir)))
> error_msg = "rec_len is smaller than minimal";
> else if (unlikely(rlen % 4 != 0))
> error_msg = "rec_len % 4 != 0";
> - else if (unlikely(rlen < ext4_dir_rec_len(de->name_len,
> - fake ? NULL : dir)))
> - error_msg = "rec_len is too small for name_len";
> else if (unlikely(next_offset > size))
> error_msg = "directory entry overrun";
> - else if (unlikely(next_offset > size - ext4_dir_rec_len(1,
> + else if (unlikely(rlen < ext4_dir_entry_len(de, size, fake ? NULL : dir)))
> + error_msg = "rec_len is too small for name_len";
> + else if (unlikely(next_offset > size - ext4_dirent_rec_len(1,
> has_csum ? NULL : dir) &&
> next_offset != size))
> error_msg = "directory entry too close to block end";
[ ... ]
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 2d6d3be4ef7df..839274c4d7fcd 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2649,11 +2649,16 @@ struct ext4_dirent_hash {
> * casefolded and encrypted need to store the hash as well, so we add room for
> * ext4_extended_dir_entry_2. For all entries related to '.' or '..' you should
> * pass NULL for dir, as those entries do not use the extra fields.
> + *
> + * For directories with the dirdata feature, extra data may follow the filename.
> + * Use ext4_dir_entry_len() to compute the length of a directory entry
> + * including any dirdata, or ext4_dirent_rec_len() directly when the total
> + * name_len (including dirdata length) is already known.
> */
> -static inline unsigned int ext4_dir_rec_len(__u8 name_len,
> +static inline unsigned int ext4_dirent_rec_len(unsigned int name_len,
> const struct inode *dir)
> {
> - int rec_len = (name_len + 8 + EXT4_DIR_ROUND);
> + unsigned int rec_len = (name_len + 8 + EXT4_DIR_ROUND);
>
> /*
> * Without dirdata, the casefold+fscrypt hash lives at a fixed position
[ ... ]
> @@ -4266,7 +4272,7 @@ static inline unsigned int ext4_dir_entry_len(struct ext4_dir_entry_2 *de,
> unsigned int rec_len = ext4_rec_len_from_disk(de->rec_len, blocksize);
> unsigned int dirdata = ext4_dirent_get_data_len(de, rec_len);
>
> - return ext4_dir_rec_len(de->name_len + dirdata, dir);
> + return ext4_dirent_rec_len(de->name_len + dirdata, dir);
> }
[ ... ]
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index b3acab0689c93..e54608950fb34 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
[ ... ]
> @@ -1902,7 +1902,7 @@ dx_move_dirents(struct inode *dir, char *from, char *to,
> while (count--) {
> struct ext4_dir_entry_2 *de = (struct ext4_dir_entry_2 *)
> (from + (map->offs<<2));
> - rec_len = ext4_dir_rec_len(de->name_len, dir);
> + rec_len = ext4_dir_entry_len(de, blocksize, dir);
>
> memcpy (to, de, rec_len);
> ((struct ext4_dir_entry_2 *) to)->rec_len =
> @@ -1935,7 +1935,7 @@ static struct ext4_dir_entry_2 *dx_pack_dirents(struct inode *dir, char *base,
> while ((char*)de < base + blocksize) {
> next = ext4_next_entry(de, blocksize);
> if (de->inode && de->name_len) {
> - rec_len = ext4_dir_rec_len(de->name_len, dir);
> + rec_len = ext4_dir_entry_len(de, blocksize, dir);
> if (de > to)
> memmove(to, de, rec_len);
> to->rec_len = ext4_rec_len_to_disk(rec_len, blocksize);
[ ... ]
> @@ -2126,7 +2127,7 @@ void ext4_insert_dentry(struct inode *dir,
>
> int nlen, rlen;
>
> - nlen = ext4_dir_rec_len(de->name_len, dir);
> + nlen = ext4_dir_entry_len(de, buf_size, dir);
> rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
> if (de->inode) {
> struct ext4_dir_entry_2 *de1 =
[Severity: Critical]
Can switching to ext4_dir_entry_len() here and widening ext4_dirent_rec_len()
cause an out-of-bounds write past bh->b_data or trigger the BUG_ON() in
ext4_rec_len_to_disk() when splitting a directory block via
do_split() -> add_dirent_to_buf() -> ext4_insert_dentry()?
In fs/ext4/ext4.h, ext4_dir_entry_len() computes the entry length using the
entry's current de->rec_len via ext4_dirent_get_data_len():
fs/ext4/ext4.h:ext4_dirent_get_data_len() {
...
{
u8 ddh_len = READ_ONCE(ddh->ddh_length);
if (ddh_len == 0 || ddh_len > rec_len - offset)
return 0;
dlen += ddh_len + (dlen == 0);
...
}
Because ext4_dirent_get_data_len() returns 0 whenever ddh_len exceeds
rec_len - offset (and does not check ext4_has_feature_dirdata()),
__ext4_check_dir_entry() in fs/ext4/dir.c will not reject an entry whose
on-disk rec_len is smaller than its dirdata length.
During a directory leaf split in fs/ext4/namei.c:do_split():
fs/ext4/namei.c:do_split() {
...
de2 = dx_move_dirents(dir, data1, data2, map + split, count - split,
blocksize);
de = dx_pack_dirents(dir, data1, blocksize);
de->rec_len = ext4_rec_len_to_disk(data1 + (blocksize - csum_size) -
(char *) de,
blocksize);
de2->rec_len = ext4_rec_len_to_disk(data2 + (blocksize - csum_size) -
(char *) de2,
blocksize);
...
}
If the final entry de in the target split half has a small rec_len (for
example, rec_len = 12, name_len = 1, file_type = 0x11) with a larger
ddh_length byte stored at offset 10 (still inside the 12-byte entry),
ext4_dirent_get_data_len(de, 12) returns 0 during __ext4_check_dir_entry(),
dx_move_dirents(), and dx_pack_dirents(), copying all 12 bytes.
Then do_split() expands de->rec_len to cover the remainder of the block and
passes de to add_dirent_to_buf(), which skips ext4_find_dest_de() when de is
non-NULL and calls ext4_insert_dentry() directly:
fs/ext4/namei.c:ext4_insert_dentry() {
int nlen, rlen;
nlen = ext4_dir_entry_len(de, buf_size, dir);
rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
if (de->inode) {
struct ext4_dir_entry_2 *de1 =
(struct ext4_dir_entry_2 *)((char *)de + nlen);
de1->rec_len = ext4_rec_len_to_disk(rlen - nlen, buf_size);
de->rec_len = ext4_rec_len_to_disk(nlen, buf_size);
de = de1;
}
...
memcpy(de->name, fname_name(fname), fname_len(fname));
}
When ext4_insert_dentry() recomputes nlen = ext4_dir_entry_len(de, buf_size,
dir) with the expanded de->rec_len, ddh_len <= rec_len - offset can now
succeed, growing nlen up to rlen (or rlen + 8 if ext4_hash_in_dirent(dir) &&
!ext4_has_feature_dirdata(dir->i_sb)). In addition, because
ext4_dirent_rec_len() now takes unsigned int name_len, an entry with multiple
valid dirdata fields can be up to 1284 bytes, which on 1024-byte or 2048-byte
block filesystems can also leave rlen - nlen in do_split() smaller than the
new entry being inserted.
Since ext4_insert_dentry() does not check that rlen - nlen is large enough to
hold the new entry, wouldn't nlen > rlen underflow rlen - nlen and hit BUG_ON
in ext4_rec_len_to_disk(), or nlen <= rlen advance de1 to the end of the
block buffer so that memcpy(de->name, ...) writes past the end of bh->b_data?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029122413.230409-1-ablagodarenko@thelustrecollective.com?part=7
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v13 08/11] ext4: dirdata feature
2026-10-29 12:23 [PATCH v13 00/11] Data in direntry (dirdata) feature Artem Blagodarenko
` (6 preceding siblings ...)
2026-10-29 12:24 ` [PATCH v13 07/11] ext4: rename ext4_dir_rec_len() and clarify dirdata usage Artem Blagodarenko
@ 2026-10-29 12:24 ` Artem Blagodarenko
2026-10-29 12:51 ` sashiko-bot
2026-10-29 12:24 ` [PATCH v13 09/11] ext4: add dirdata LUFID support for directory entry rename Artem Blagodarenko
` (2 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Artem Blagodarenko @ 2026-10-29 12:24 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, jack, adilger.kernel, ablagodarenko, Artem Blagodarenko,
Pravin Shelar, Andreas Dilger
From: Artem Blagodarenko <artem.blagodarenko@gmail.com>
When fscrypt and casefold are enabled together for a directory,
all ext4_dir_entry[_2] in that directory store a n 8-byte hash
of the filename after 'name' between 'name_len' and 'rec_len'.
However, there is no clear indication there is important data
stored in these bytes, which are only for padding and alignment
in other directory entries. This adds complexity to code handling
the on-disk directory entries, and there is no provision for other
metadata to be stored in each dir entry after 'name'.
The dirdata feature adds a mechanism to store multiple metadata
entries in each dir entry after 'name' (including the fchash).
The unused high 4 bits of 'file_type' are used to indicate whether
additional data fields are stored after 'name'. If a bit is set,
the corresponding dirdata record is present, starting after a NUL
filename terminator. If present, a record starts with a 1-byte
length (including the length byte itself) and the data immediately
follows the length byte without any alignment.
This allows up to four different dirdata records to be stored in
each entry, and allows unhandled record bytes to be skipped without
having to process the contents, providing forward compatibility.
If and when the fourth and last dirdata record is needed, it is
recommended to further subdivide it into sub-records, with
the first byte being the total length, and then there being a
second byte that gives the sub-record length, etc. as long as
the total record length is less than 255 bytes. However, this
would not affect compatibility with the current code since the
record length would allow it to be skipped without processing.
Signed-off-by: Pravin Shelar <pravin.shelar@sun.com>
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
---
fs/ext4/dir.c | 22 +++++++--
fs/ext4/ext4.h | 43 ++++++++++++++---
fs/ext4/fast_commit.c | 1 +
fs/ext4/fast_commit.h | 1 +
fs/ext4/inline.c | 34 ++++++++++----
fs/ext4/namei.c | 107 ++++++++++++++++++++++++++++++------------
fs/ext4/sysfs.c | 2 +
7 files changed, 163 insertions(+), 47 deletions(-)
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 38911ae2bcfc..15816f2a5f47 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -483,27 +483,43 @@ void ext4_htree_free_dir_info(struct dir_private_info *p)
* The decrypted filename is passed in via ent_name. parameter.
*/
int ext4_htree_store_dirent(struct file *dir_file, __u32 hash,
- __u32 minor_hash,
+ __u32 minor_hash, int buf_size,
struct ext4_dir_entry_2 *dirent,
struct fscrypt_str *ent_name)
{
struct rb_node **p, *parent = NULL;
struct fname *fname, *new_fn;
struct dir_private_info *info;
+ int extra_data = 0;
info = dir_file->private_data;
p = &info->root.rb_node;
/* Create and allocate the fname structure */
- new_fn = kzalloc_flex(*new_fn, name, ent_name->len + 1);
+ if (dirent->file_type & ~EXT4_FT_MASK) {
+ unsigned int rec_len =
+ ext4_rec_len_from_disk(dirent->rec_len, buf_size);
+ extra_data = ext4_dirent_get_data_len(dirent, rec_len);
+ }
+
+ /* exta_data contains +1 byte for NULL */
+ new_fn = kzalloc_flex(*new_fn, name, ent_name->len +
+ (extra_data ? extra_data : 1));
if (!new_fn)
return -ENOMEM;
new_fn->hash = hash;
new_fn->minor_hash = minor_hash;
new_fn->inode = le32_to_cpu(dirent->inode);
- new_fn->name_len = ent_name->len;
new_fn->file_type = dirent->file_type;
+ new_fn->name_len = ent_name->len;
memcpy(new_fn->name, ent_name->name, ent_name->len);
+ if (extra_data)
+ /* Dirdata bytes lie past the __counted_by(name_len) bound but
+ * within the kzalloc_flex allocation; use unsafe_memcpy to
+ * avoid a FORTIFY_SOURCE false positive. */
+ unsafe_memcpy(new_fn->name + ent_name->len,
+ dirent->name + dirent->name_len, extra_data,
+ /* within kzalloc_flex allocation */);
while (*p) {
parent = *p;
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 839274c4d7fc..aea75a320ceb 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2369,6 +2369,7 @@ EXT4_FEATURE_INCOMPAT_FUNCS(casefold, CASEFOLD)
EXT4_FEATURE_INCOMPAT_FLEX_BG| \
EXT4_FEATURE_INCOMPAT_EA_INODE| \
EXT4_FEATURE_INCOMPAT_MMP | \
+ EXT4_FEATURE_INCOMPAT_DIRDATA | \
EXT4_FEATURE_INCOMPAT_INLINE_DATA | \
EXT4_FEATURE_INCOMPAT_ENCRYPT | \
EXT4_FEATURE_INCOMPAT_CASEFOLD | \
@@ -2633,6 +2634,18 @@ struct ext4_dirent_hash {
struct ext4_dir_entry_hash dh_hash;
} __packed;
+static inline
+struct ext4_dirent_fid *ext4_dentry_get_fid(struct super_block *sb,
+ struct ext4_dentry_param *p)
+{
+ if (!ext4_has_feature_dirdata(sb))
+ return NULL;
+ if (p && p->edp_magic == EXT4_LUFID_MAGIC)
+ return (struct ext4_dirent_fid *)p->edp_dfid;
+
+ return NULL;
+}
+
#define EXT4_FT_DIR_CSUM 0xDE
/*
@@ -2756,6 +2769,8 @@ struct ext4_filename {
#if IS_ENABLED(CONFIG_UNICODE)
struct qstr cf_name;
#endif
+ /* LUFID/dirdata payload to embed in the new directory entry, or NULL */
+ struct ext4_dirent_fid *dfid;
};
#define fname_name(p) ((p)->disk_name.name)
@@ -3067,7 +3082,7 @@ extern int __ext4_check_dir_entry(const char *, unsigned int, struct inode *,
unlikely(__ext4_check_dir_entry(__func__, __LINE__, (dir), (filp), \
(de), (bh), (buf), (size), (offset)))
extern int ext4_htree_store_dirent(struct file *dir_file, __u32 hash,
- __u32 minor_hash,
+ __u32 minor_hash, int buf_size,
struct ext4_dir_entry_2 *dirent,
struct fscrypt_str *ent_name);
extern void ext4_htree_free_dir_info(struct dir_private_info *p);
@@ -3076,10 +3091,18 @@ extern int ext4_find_dest_de(struct inode *dir, struct buffer_head *bh,
struct ext4_filename *fname,
struct ext4_dir_entry_2 **dest_de,
int dlen);
-void ext4_insert_dentry(struct inode *dir, struct inode *inode,
- struct ext4_dir_entry_2 *de,
- int buf_size,
- struct ext4_filename *fname);
+void ext4_insert_dentry_data(struct inode *dir, struct inode *inode,
+ struct ext4_dir_entry_2 *de,
+ int buf_size,
+ struct ext4_filename *fname,
+ void *data);
+static inline void ext4_insert_dentry(struct inode *dir, struct inode *inode,
+ struct ext4_dir_entry_2 *de,
+ int buf_size,
+ struct ext4_filename *fname)
+{
+ ext4_insert_dentry_data(dir, inode, de, buf_size, fname, NULL);
+}
static inline void ext4_update_dx_flag(struct inode *inode)
{
if (!ext4_has_feature_dir_index(inode->i_sb) &&
@@ -3336,8 +3359,14 @@ extern int ext4_ext_migrate(struct inode *);
extern int ext4_ind_migrate(struct inode *inode);
/* namei.c */
-extern int ext4_init_new_dir(handle_t *handle, struct inode *dir,
- struct inode *inode);
+extern int ext4_init_new_dir_data(handle_t *handle, struct inode *dir,
+ struct inode *inode,
+ const void *data1, const void *data2);
+static inline int ext4_init_new_dir(handle_t *handle, struct inode *dir,
+ struct inode *inode)
+{
+ return ext4_init_new_dir_data(handle, dir, inode, NULL, NULL);
+}
extern int ext4_dirblock_csum_verify(struct inode *inode,
struct buffer_head *bh);
extern int ext4_htree_fill_tree(struct file *dir_file, __u32 start_hash,
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 0cac890cf370..876a863d6c01 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -2686,6 +2686,7 @@ static const char * const fc_ineligible_reasons[] = {
[EXT4_FC_REASON_MIGRATE] = "Inode format migration",
[EXT4_FC_REASON_VERITY] = "fs-verity enable",
[EXT4_FC_REASON_MOVE_EXT] = "Move extents",
+ [EXT4_FC_REASON_DIRDATA] = "Dirdata LUFID (not recorded by fast-commit)",
};
int ext4_fc_info_show(struct seq_file *seq, void *v)
diff --git a/fs/ext4/fast_commit.h b/fs/ext4/fast_commit.h
index 2f77a37fb101..899faeb4e6e0 100644
--- a/fs/ext4/fast_commit.h
+++ b/fs/ext4/fast_commit.h
@@ -100,6 +100,7 @@ enum {
EXT4_FC_REASON_MIGRATE,
EXT4_FC_REASON_VERITY,
EXT4_FC_REASON_MOVE_EXT,
+ EXT4_FC_REASON_DIRDATA,
EXT4_FC_REASON_MAX
};
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index bdf6f45a3de4..d3dda1d65d06 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -982,14 +982,20 @@ static int ext4_add_dirent_to_inline(handle_t *handle,
struct ext4_iloc *iloc,
void *inline_start, int inline_size)
{
- int err;
+ int err, dlen = 0;
struct ext4_dir_entry_2 *de;
+ struct ext4_dirent_fid *dfid = NULL;
+
+ /* fname->dfid was set in ext4_add_entry() */
+ dfid = fname->dfid;
+ if (dfid)
+ dlen = dfid->df_header.ddh_length;
+ if (ext4_hash_in_dirent(dir) &&
+ ext4_has_feature_dirdata(dir->i_sb))
+ dlen += sizeof(struct ext4_dirent_hash);
- /* 'dlen' reserves space for dirdata extensions (LUFID, CFHASH) in the
- * slot. The dirdata types are defined in the next patch; until then
- * dlen stays 0 and ext4_find_dest_de() ignores dirdata overhead. */
err = ext4_find_dest_de(dir, iloc->bh, inline_start,
- inline_size, fname, &de, 0);
+ inline_size, fname, &de, dlen);
if (err)
return err;
@@ -998,7 +1004,7 @@ static int ext4_add_dirent_to_inline(handle_t *handle,
EXT4_JTR_NONE);
if (err)
return err;
- ext4_insert_dentry(dir, inode, de, inline_size, fname);
+ ext4_insert_dentry_data(dir, inode, de, inline_size, fname, dfid);
ext4_show_inline_dir(dir, iloc->bh, inline_start, inline_size);
@@ -1276,6 +1282,7 @@ int ext4_inlinedir_to_tree(struct file *dir_file,
int err = 0, count = 0;
unsigned int parent_ino;
int pos;
+ unsigned int de_len;
struct ext4_dir_entry_2 *de;
struct inode *inode = file_inode(dir_file);
int ret, inline_size = 0;
@@ -1337,14 +1344,24 @@ int ext4_inlinedir_to_tree(struct file *dir_file,
de = &fake;
pos = EXT4_INLINE_DOTDOT_SIZE;
} else {
+ if (pos + EXT4_BASE_DIR_LEN > inline_size) {
+ ret = count;
+ goto out;
+ }
de = (struct ext4_dir_entry_2 *)(dir_buf + pos);
- pos += ext4_rec_len_from_disk(de->rec_len, inline_size);
+ de_len = ext4_rec_len_from_disk(de->rec_len, inline_size);
+ if (de_len < EXT4_BASE_DIR_LEN ||
+ pos + de_len > (unsigned int)inline_size) {
+ ret = count;
+ goto out;
+ }
if (ext4_check_dir_entry(inode, dir_file, de,
iloc.bh, dir_buf,
inline_size, pos)) {
ret = count;
goto out;
}
+ pos += de_len;
}
if (ext4_hash_in_dirent(dir)) {
@@ -1366,7 +1383,8 @@ int ext4_inlinedir_to_tree(struct file *dir_file,
tmp_str.name = de->name;
tmp_str.len = de->name_len;
err = ext4_htree_store_dirent(dir_file, hinfo->hash,
- hinfo->minor_hash, de, &tmp_str);
+ hinfo->minor_hash, inline_size,
+ de, &tmp_str);
if (err) {
ret = err;
goto out;
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index e54608950fb3..30bbf6a56bf6 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -402,23 +402,32 @@ static struct dx_countlimit *get_dx_countlimit(struct inode *inode,
{
struct ext4_dir_entry_2 *de;
struct dx_root_info *root;
- int count_offset;
+ int count_offset, dotdot_rec_len;
int blocksize = EXT4_BLOCK_SIZE(inode->i_sb);
unsigned int rlen = ext4_rec_len_from_disk(dirent->rec_len, blocksize);
- if (rlen == blocksize)
+ if (rlen == blocksize) {
count_offset = sizeof(struct dx_node);
- else if (rlen == 12) {
- de = (struct ext4_dir_entry_2 *)(((void *)dirent) + 12);
- if (ext4_rec_len_from_disk(de->rec_len, blocksize) != blocksize - 12)
+ } else {
+ if (rlen < EXT4_BASE_DIR_LEN ||
+ rlen + EXT4_BASE_DIR_LEN > blocksize)
+ return NULL;
+ de = (struct ext4_dir_entry_2 *)(((char *)dirent) + rlen);
+ if (ext4_rec_len_from_disk(de->rec_len, blocksize) !=
+ (unsigned int)(blocksize - rlen))
+ return NULL;
+ /* de->rec_len covers whole dx_root block, calculate actual length.
+ * This is the '..' entry, which never carries the casefold+fscrypt
+ * hash, so pass NULL for dir regardless of the directory's flags */
+ dotdot_rec_len = ext4_dir_entry_len(de, blocksize, NULL);
+ if (rlen + dotdot_rec_len + sizeof(struct dx_root_info) > blocksize)
return NULL;
- root = (struct dx_root_info *)(((void *)de + 12));
+ root = (struct dx_root_info *)(((char *)de + dotdot_rec_len));
if (root->reserved_zero ||
root->info_length != sizeof(struct dx_root_info))
return NULL;
- count_offset = 32;
- } else
- return NULL;
+ count_offset = root->info_length + rlen + dotdot_rec_len;
+ }
if (offset)
*offset = count_offset;
@@ -728,7 +737,9 @@ static struct stats dx_show_leaf(struct inode *dir,
(unsigned) ((char *) de - base));
#endif
}
- space += ext4_dirent_rec_len(de->name_len, dir);
+ if ((char *)de + EXT4_BASE_DIR_LEN > base + size)
+ break;
+ space += ext4_dir_entry_len(de, size, dir);
names++;
}
de = ext4_next_entry(de, size);
@@ -1157,8 +1168,8 @@ static int htree_dirblock_to_tree(struct file *dir_file,
tmp_str.name = de->name;
tmp_str.len = de->name_len;
err = ext4_htree_store_dirent(dir_file,
- hinfo->hash, hinfo->minor_hash, de,
- &tmp_str);
+ hinfo->hash, hinfo->minor_hash,
+ dir->i_sb->s_blocksize, de, &tmp_str);
} else {
int save_len = fname_crypto_str.len;
struct fscrypt_str de_name = FSTR_INIT(de->name,
@@ -1173,8 +1184,9 @@ static int htree_dirblock_to_tree(struct file *dir_file,
goto errout;
}
err = ext4_htree_store_dirent(dir_file,
- hinfo->hash, hinfo->minor_hash, de,
- &fname_crypto_str);
+ hinfo->hash, hinfo->minor_hash,
+ dir->i_sb->s_blocksize, de,
+ &fname_crypto_str);
fname_crypto_str.len = save_len;
}
if (err != 0) {
@@ -1252,6 +1264,7 @@ int ext4_htree_fill_tree(struct file *dir_file, __u32 start_hash,
tmp_str.name = de->name;
tmp_str.len = de->name_len;
err = ext4_htree_store_dirent(dir_file, 0, 0,
+ dir->i_sb->s_blocksize,
de, &tmp_str);
if (err != 0)
goto errout;
@@ -1263,6 +1276,7 @@ int ext4_htree_fill_tree(struct file *dir_file, __u32 start_hash,
tmp_str.name = de->name;
tmp_str.len = de->name_len;
err = ext4_htree_store_dirent(dir_file, 2, 0,
+ dir->i_sb->s_blocksize,
de, &tmp_str);
if (err != 0)
goto errout;
@@ -2091,7 +2105,11 @@ int ext4_find_dest_de(struct inode *dir, struct buffer_head *bh,
int dlen)
{
struct ext4_dir_entry_2 *de;
- unsigned short reclen = ext4_dirent_rec_len(fname_len(fname) + dlen, dir);
+ /* When dirdata extensions are present, a NUL gap byte sits between
+ * the filename and the first extension; account for it so the found
+ * slot is always large enough for ext4_dirdata_set() to write into. */
+ unsigned short reclen = ext4_dirent_rec_len(fname_len(fname) + dlen +
+ (dlen ? 1 : 0), dir);
int nlen, rlen;
unsigned int offset = 0;
char *top;
@@ -2118,13 +2136,10 @@ int ext4_find_dest_de(struct inode *dir, struct buffer_head *bh,
return 0;
}
-void ext4_insert_dentry(struct inode *dir,
- struct inode *inode,
- struct ext4_dir_entry_2 *de,
- int buf_size,
- struct ext4_filename *fname)
+void ext4_insert_dentry_data(struct inode *dir, struct inode *inode,
+ struct ext4_dir_entry_2 *de, int buf_size,
+ struct ext4_filename *fname, void *data)
{
-
int nlen, rlen;
nlen = ext4_dir_entry_len(de, buf_size, dir);
@@ -2141,6 +2156,10 @@ void ext4_insert_dentry(struct inode *dir,
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));
+ /* 'data' (the LUFID payload) and the CFHASH are written by
+ * ext4_dirdata_set(), which is introduced in the next patch that adds
+ * the dirdata set/get helpers. Until then, only the casefold hash is
+ * stored at the legacy fixed offset for encrypted+casefolded dirs. */
if (ext4_hash_in_dirent(dir)) {
struct dx_hash_info *hinfo = &fname->hinfo;
@@ -2165,14 +2184,28 @@ static int add_dirent_to_buf(handle_t *handle, struct ext4_filename *fname,
{
unsigned int blocksize = dir->i_sb->s_blocksize;
int csum_size = 0;
- int err, err2;
+ int err, err2, dlen = 0;
+ struct ext4_dirent_fid *dfid;
if (ext4_has_feature_metadata_csum(inode->i_sb))
csum_size = sizeof(struct ext4_dir_entry_tail);
+ /* dfid is resolved once by ext4_add_entry() and stored in fname so
+ * that all add_dirent_to_buf() calls within the same add operation
+ * use a consistent value without touching the shared i_dirdata field. */
+ dfid = fname->dfid;
if (!de) {
+ if (dfid)
+ dlen = dfid->df_header.ddh_length;
+ /* ext4_dirent_rec_len() does not add CFHASH size when dirdata is
+ * enabled (it assumes the caller included it in name_len). Add
+ * it here so ext4_find_dest_de() allocates a slot large enough
+ * for ext4_dirdata_set() to write the hash extension. */
+ if (ext4_hash_in_dirent(dir) &&
+ ext4_has_feature_dirdata(dir->i_sb))
+ dlen += sizeof(struct ext4_dirent_hash);
err = ext4_find_dest_de(dir, bh, bh->b_data,
- blocksize - csum_size, fname, &de, 0);
+ blocksize - csum_size, fname, &de, dlen);
if (err)
return err;
}
@@ -2185,7 +2218,7 @@ static int add_dirent_to_buf(handle_t *handle, struct ext4_filename *fname,
}
/* By now the buffer is marked for journaling */
- ext4_insert_dentry(dir, inode, de, blocksize, fname);
+ ext4_insert_dentry_data(dir, inode, de, blocksize, fname, dfid);
/*
* XXX shouldn't update any times until successful
@@ -2412,7 +2445,8 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
*/
static int __ext4_add_entry(handle_t *handle, struct inode *dir,
const struct qstr *d_name,
- struct inode *inode)
+ struct inode *inode,
+ struct ext4_dentry_param *edp)
{
struct buffer_head *bh = NULL;
struct ext4_dir_entry_2 *de;
@@ -2437,6 +2471,13 @@ static int __ext4_add_entry(handle_t *handle, struct inode *dir,
if (retval)
return retval;
+ /* Resolve the LUFID payload once here and store it in fname so every
+ * add_dirent_to_buf() call in this add operation uses the same value
+ * without touching the shared EXT4_I(inode)->i_dirdata field. */
+ fname.dfid = ext4_dentry_get_fid(sb, edp);
+ if (fname.dfid)
+ ext4_fc_mark_ineligible(sb, EXT4_FC_REASON_DIRDATA, handle);
+
if (ext4_has_inline_data(dir)) {
retval = ext4_try_add_inline_entry(handle, &fname, dir, inode);
if (retval < 0)
@@ -2521,7 +2562,8 @@ static int ext4_add_entry(handle_t *handle, struct dentry *dentry,
if (fscrypt_is_nokey_name(dentry))
return -ENOKEY;
- return __ext4_add_entry(handle, dir, &dentry->d_name, inode);
+ return __ext4_add_entry(handle, dir, &dentry->d_name, inode,
+ dentry->d_fsdata);
}
/*
@@ -3029,8 +3071,9 @@ int ext4_init_dirblock(handle_t *handle, struct inode *inode,
return ext4_handle_dirty_dirblock(handle, inode, bh);
}
-int ext4_init_new_dir(handle_t *handle, struct inode *dir,
- struct inode *inode)
+int ext4_init_new_dir_data(handle_t *handle, struct inode *dir,
+ struct inode *inode,
+ const void *data1, const void *data2)
{
struct buffer_head *dir_block = NULL;
ext4_lblk_t block = 0;
@@ -3531,10 +3574,16 @@ int __ext4_link(struct inode *dir, struct inode *inode,
if (IS_DIRSYNC(dir))
ext4_handle_sync(handle);
+ /* Fast-commit does not record dirdata in its journal format; force a
+ * full journal commit so the LUFID survives crash+replay. */
+ if (ext4_dentry_get_fid(dir->i_sb, dentry ? dentry->d_fsdata : NULL))
+ ext4_fc_mark_ineligible(dir->i_sb, EXT4_FC_REASON_DIRDATA, handle);
+
inode_set_ctime_current(inode);
ext4_inc_count(inode);
- err = __ext4_add_entry(handle, dir, d_name, inode);
+ err = __ext4_add_entry(handle, dir, d_name, inode,
+ dentry ? dentry->d_fsdata : NULL);
if (!err) {
err = ext4_mark_inode_dirty(handle, inode);
/* this can happen only for tmpfile being
diff --git a/fs/ext4/sysfs.c b/fs/ext4/sysfs.c
index 923b375e017f..80074fb15ee9 100644
--- a/fs/ext4/sysfs.c
+++ b/fs/ext4/sysfs.c
@@ -362,6 +362,7 @@ EXT4_ATTR_FEATURE(verity);
#endif
EXT4_ATTR_FEATURE(metadata_csum_seed);
EXT4_ATTR_FEATURE(fast_commit);
+EXT4_ATTR_FEATURE(dirdata);
#if IS_ENABLED(CONFIG_UNICODE) && defined(CONFIG_FS_ENCRYPTION)
EXT4_ATTR_FEATURE(encrypted_casefold);
#endif
@@ -385,6 +386,7 @@ static struct attribute *ext4_feat_attrs[] = {
#endif
ATTR_LIST(metadata_csum_seed),
ATTR_LIST(fast_commit),
+ ATTR_LIST(dirdata),
#if IS_ENABLED(CONFIG_UNICODE) && defined(CONFIG_FS_ENCRYPTION)
ATTR_LIST(encrypted_casefold),
#endif
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v13 08/11] ext4: dirdata feature
2026-10-29 12:24 ` [PATCH v13 08/11] ext4: dirdata feature Artem Blagodarenko
@ 2026-10-29 12:51 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-10-29 12:51 UTC (permalink / raw)
To: Artem Blagodarenko, linux-ext4; +Cc: tytso
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] ext4: missing legacy hash size in ext4_dirent_rec_len() with dirdata
- [High] ext4: out-of-bounds read of htree root dirents in ext4_htree_store_dirent()
- [High] ext4: out-of-bounds read in ext4_dx_csum_verify() via get_dx_countlimit()
- [Medium] ext4: uninitialized stack read of fake dirent in ext4_htree_store_dirent()
--
commit 31cebce918ae1e358449a063cc4a21909275ec8a
Author: Artem Blagodarenko <artem.blagodarenko@gmail.com>
ext4: dirdata feature
This commit enables the EXT4_FEATURE_INCOMPAT_DIRDATA feature flag and updates
directory entry handling across htree, inline directory, and directory entry
insertion paths to account for variable-length dirdata records stored after
the filename.
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index 38911ae2bcfc0..15816f2a5f472 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -483,27 +483,43 @@ void ext4_htree_free_dir_info(struct dir_private_info *p)
> * The decrypted filename is passed in via ent_name. parameter.
> */
> int ext4_htree_store_dirent(struct file *dir_file, __u32 hash,
> - __u32 minor_hash,
> + __u32 minor_hash, int buf_size,
> struct ext4_dir_entry_2 *dirent,
> struct fscrypt_str *ent_name)
> {
> struct rb_node **p, *parent = NULL;
> struct fname *fname, *new_fn;
> struct dir_private_info *info;
> + int extra_data = 0;
>
> info = dir_file->private_data;
> p = &info->root.rb_node;
>
> /* Create and allocate the fname structure */
> - new_fn = kzalloc_flex(*new_fn, name, ent_name->len + 1);
> + if (dirent->file_type & ~EXT4_FT_MASK) {
> + unsigned int rec_len =
> + ext4_rec_len_from_disk(dirent->rec_len, buf_size);
> + extra_data = ext4_dirent_get_data_len(dirent, rec_len);
> + }
[Severity: Medium]
Can this read uninitialized stack memory from fake.file_type and fake.name[]
when called from ext4_inlinedir_to_tree()?
In fs/ext4/inline.c, ext4_inlinedir_to_tree() declares struct ext4_dir_entry_2
fake on the stack without initializing it, and populates it for the "." and
".." entries:
fs/ext4/inline.c:ext4_inlinedir_to_tree() {
struct ext4_dir_entry_2 fake;
...
if (pos == 0) {
fake.inode = cpu_to_le32(inode->i_ino);
fake.name_len = 1;
memcpy(fake.name, ".", 2);
fake.rec_len = ext4_rec_len_to_disk(
ext4_dirent_rec_len(fake.name_len, NULL),
inline_size);
ext4_set_de_type(inode->i_sb, &fake, S_IFDIR);
de = &fake;
pos = EXT4_INLINE_DOTDOT_OFFSET;
}
However, ext4_set_de_type() only writes de->file_type when the filetype
feature is enabled:
fs/ext4/ext4.h:ext4_set_de_type() {
if (ext4_has_feature_filetype(sb))
de->file_type = ext4_type_by_mode[(mode & S_IFMT)>>S_SHIFT];
}
On a filesystem with inline_data and dir_index enabled but without
EXT4_FEATURE_INCOMPAT_FILETYPE, fake.file_type and fake.name[2..3] remain
uninitialized on the stack when ext4_inlinedir_to_tree() passes de = &fake to
ext4_htree_store_dirent().
If any upper bit of the uninitialized fake.file_type is non-zero,
ext4_dirent_get_data_len(&fake, 12) reads ddh->ddh_length from uninitialized
stack bytes fake.name[2..3] and unsafe_memcpy() copies those uninitialized
bytes into new_fn->name.
The later commit in the series ("ext4: add dirdata set/get helpers") gates
this check on ext4_has_feature_dirdata() and also calls ext4_dirdata_get() on
de in ext4_inlinedir_to_tree(), but neither ext4_feature_set_ok() nor
ext4_dirdata_get() requires EXT4_FEATURE_INCOMPAT_FILETYPE when
EXT4_FEATURE_INCOMPAT_DIRDATA is enabled, and fake.file_type is still left
uninitialized before ext4_set_de_type().
Should ext4_inlinedir_to_tree() initialize fake.file_type = EXT4_FT_UNKNOWN
before calling ext4_set_de_type(), matching ext4_insert_dentry_data()?
[ ... ]
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index e54608950fb34..30bbf6a56bf64 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -402,23 +402,32 @@ static struct dx_countlimit *get_dx_countlimit(struct inode *inode,
> {
> struct ext4_dir_entry_2 *de;
> struct dx_root_info *root;
> - int count_offset;
> + int count_offset, dotdot_rec_len;
> int blocksize = EXT4_BLOCK_SIZE(inode->i_sb);
> unsigned int rlen = ext4_rec_len_from_disk(dirent->rec_len, blocksize);
>
> - if (rlen == blocksize)
> + if (rlen == blocksize) {
> count_offset = sizeof(struct dx_node);
> - else if (rlen == 12) {
> - de = (struct ext4_dir_entry_2 *)(((void *)dirent) + 12);
> - if (ext4_rec_len_from_disk(de->rec_len, blocksize) != blocksize - 12)
> + } else {
> + if (rlen < EXT4_BASE_DIR_LEN ||
> + rlen + EXT4_BASE_DIR_LEN > blocksize)
> + return NULL;
> + de = (struct ext4_dir_entry_2 *)(((char *)dirent) + rlen);
> + if (ext4_rec_len_from_disk(de->rec_len, blocksize) !=
> + (unsigned int)(blocksize - rlen))
> return NULL;
> - root = (struct dx_root_info *)(((void *)de + 12));
> + /* de->rec_len covers whole dx_root block, calculate actual length.
> + * This is the '..' entry, which never carries the casefold+fscrypt
> + * hash, so pass NULL for dir regardless of the directory's flags */
> + dotdot_rec_len = ext4_dir_entry_len(de, blocksize, NULL);
> + if (rlen + dotdot_rec_len + sizeof(struct dx_root_info) > blocksize)
> + return NULL;
> + root = (struct dx_root_info *)(((char *)de + dotdot_rec_len));
> if (root->reserved_zero ||
> root->info_length != sizeof(struct dx_root_info))
> return NULL;
> - count_offset = 32;
> - } else
> - return NULL;
> + count_offset = root->info_length + rlen + dotdot_rec_len;
> + }
>
> if (offset)
> *offset = count_offset;
[Severity: High]
Does get_dx_countlimit() allow a 4-byte out-of-bounds read past the end of
bh->b_data in ext4_dx_csum_verify() and ext4_dx_csum_set()?
The bounds check in get_dx_countlimit() checks:
if (rlen + dotdot_rec_len + sizeof(struct dx_root_info) > blocksize)
return NULL;
without accounting for sizeof(struct dx_countlimit) (unlike dx_get_dx_info(),
which includes sizeof(struct dx_entry) in its upper-bound check).
On a corrupted or crafted image where rlen + dotdot_rec_len +
sizeof(struct dx_root_info) == blocksize, get_dx_countlimit() sets
count_offset = blocksize and returns (struct dx_countlimit *)(((void *)dirent)
+ blocksize), which points directly past the end of the directory block
buffer.
Both ext4_dx_csum_verify() and ext4_dx_csum_set() immediately dereference
c->limit and c->count before validating count_offset against blocksize:
fs/ext4/namei.c:ext4_dx_csum_verify() {
c = get_dx_countlimit(inode, dirent, &count_offset);
if (!c) {
EXT4_ERROR_INODE(inode, "dir seems corrupt? Run e2fsck -D.");
return 0;
}
limit = le16_to_cpu(c->limit);
count = le16_to_cpu(c->count);
if (count_offset + (limit * sizeof(struct dx_entry)) >
EXT4_BLOCK_SIZE(inode->i_sb) - sizeof(struct ext4_dir_entry_tail))
...
}
Should the check in get_dx_countlimit() include sizeof(struct dx_countlimit)
so that the returned struct dx_countlimit pointer is guaranteed to lie within
the block?
[ ... ]
> @@ -1252,6 +1264,7 @@ int ext4_htree_fill_tree(struct file *dir_file, __u32 start_hash,
> tmp_str.name = de->name;
> tmp_str.len = de->name_len;
> err = ext4_htree_store_dirent(dir_file, 0, 0,
> + dir->i_sb->s_blocksize,
> de, &tmp_str);
> if (err != 0)
> goto errout;
> @@ -1263,6 +1276,7 @@ int ext4_htree_fill_tree(struct file *dir_file, __u32 start_hash,
> tmp_str.name = de->name;
> tmp_str.len = de->name_len;
> err = ext4_htree_store_dirent(dir_file, 2, 0,
> + dir->i_sb->s_blocksize,
> de, &tmp_str);
> if (err != 0)
> goto errout;
[Severity: High]
Could ext4_htree_fill_tree() and ext4_htree_store_dirent() read past the end
of frames[0].bh->b_data when storing the "." and ".." entries from an htree
root block?
In dx_probe() -> dx_get_dx_info(), the code steps from "." to ".." using
ext4_dir_entry_len():
fs/ext4/namei.c:dx_get_dx_info() {
if (ext4_rec_len_from_disk(((struct ext4_dir_entry_2 *)de_buf)->rec_len,
blocksize) < EXT4_BASE_DIR_LEN)
return ERR_PTR(-EFSCORRUPTED);
de_buf += ext4_dir_entry_len(de_buf, blocksize, NULL);
}
By contrast, get_dx_countlimit() and ext4_htree_fill_tree() advance from "."
to ".." using dot_de->rec_len (via ext4_next_entry(de, dir->i_sb->s_blocksize))
without verifying that dot_de->rec_len matches ext4_dir_entry_len(dot_de,
blocksize, NULL).
Even with the additional rlen checks added in the later commit ("ext4: Add
EXT4_IOC_SET_LUFID ioctl for setting LUFID on directory entries"),
dx_get_dx_info() permits rlen <= blocksize on "." and validates the ".." entry
and dx_root_info at offset ext4_dir_entry_len(dot_de) (for example, offset 12)
rather than at offset rlen.
As a result, on a crafted image where dot_de has ext4_dir_entry_len(dot_de) ==
12 (with valid ".." and dx_root_info at offsets 12 and 24) but dot_de->rec_len
decodes to rlen == blocksize (or blocksize - 4), dx_probe() succeeds and
ext4_htree_fill_tree() then advances de = ext4_next_entry(de, s_blocksize) to
frames[0].bh->b_data + rlen, reading de->name_len, de->inode, de->file_type,
and de->rec_len out of bounds.
Similarly, if dot_de->rec_len decodes to an offset near the end of the block
(such as 4000), ext4_htree_fill_tree() passes "." and ".." to
ext4_htree_store_dirent() without calling ext4_check_dir_entry(). In
ext4_htree_store_dirent(), rec_len = ext4_rec_len_from_disk(dirent->rec_len,
buf_size) is passed directly to ext4_dirent_get_data_len(dirent, rec_len)
without checking that (char *)dirent + rec_len or dirent->name +
dirent->name_len fits within frames[0].bh->b_data + s_blocksize, allowing
memcpy() and unsafe_memcpy() to read past the end of the block buffer.
[ ... ]
> @@ -2118,13 +2136,10 @@ int ext4_find_dest_de(struct inode *dir, struct buffer_head *bh,
> return 0;
> }
>
> -void ext4_insert_dentry(struct inode *dir,
> - struct inode *inode,
> - struct ext4_dir_entry_2 *de,
> - int buf_size,
> - struct ext4_filename *fname)
> +void ext4_insert_dentry_data(struct inode *dir, struct inode *inode,
> + struct ext4_dir_entry_2 *de, int buf_size,
> + struct ext4_filename *fname, void *data)
> {
> -
> int nlen, rlen;
>
> nlen = ext4_dir_entry_len(de, buf_size, dir);
> @@ -2141,6 +2156,10 @@ void ext4_insert_dentry(struct inode *dir,
> 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));
> + /* 'data' (the LUFID payload) and the CFHASH are written by
> + * ext4_dirdata_set(), which is introduced in the next patch that adds
> + * the dirdata set/get helpers. Until then, only the casefold hash is
> + * stored at the legacy fixed offset for encrypted+casefolded dirs. */
> if (ext4_hash_in_dirent(dir)) {
> struct dx_hash_info *hinfo = &fname->hinfo;
>
[Severity: Critical]
Does enabling EXT4_FEATURE_INCOMPAT_DIRDATA in EXT4_FEATURE_INCOMPAT_SUPP
while ext4_dirent_rec_len() omits sizeof(struct ext4_dir_entry_hash) cause
hash corruption and 8-byte out-of-bounds reads in encrypted, casefolded
directories when EXT4_DIRENT_CFHASH is not set on an entry?
In fs/ext4/ext4.h, ext4_dirent_rec_len() only reserves
sizeof(struct ext4_dir_entry_hash) when dirdata is not enabled on the
superblock:
fs/ext4/ext4.h:ext4_dirent_rec_len() {
if (dir && ext4_hash_in_dirent(dir) &&
!ext4_has_feature_dirdata(dir->i_sb))
rec_len += sizeof(struct ext4_dir_entry_hash);
return (rec_len & ~EXT4_DIR_ROUND);
}
This commit adds EXT4_FEATURE_INCOMPAT_DIRDATA to EXT4_FEATURE_INCOMPAT_SUPP,
making ext4_has_feature_dirdata(dir->i_sb) true on mounted dirdata
filesystems.
At this commit, ext4_insert_dentry_data() still writes the casefold hash at
EXT4_DIRENT_HASHES(de) without setting EXT4_DIRENT_CFHASH in de->file_type.
Because ext4_dirent_get_data_len() returns 0 and ext4_dirent_rec_len() omits
sizeof(struct ext4_dir_entry_hash), ext4_dir_entry_len(de, buf_size, dir)
returns a length 8 bytes too small, and splitting the entry at nlen =
ext4_dir_entry_len() overwrites the 8-byte hash.
Even after the follow-up commit ("ext4: add dirdata set/get helpers") adds
ext4_dirdata_set() and ext4_dirdata_get(), all hash readers
(ext4_inlinedir_to_tree(), ext4_search_dir() -> ext4_match(), ext4_readdir(),
htree_dirblock_to_tree(), and dx_make_map()) fall back to reading
EXT4_DIRENT_HASH(de) and EXT4_DIRENT_MINOR_HASH(de) at the legacy fixed offset
whenever EXT4_DIRENT_CFHASH is not set in de->file_type (or when
ext4_dirdata_get() returns an error).
However, __ext4_check_dir_entry() and ext4_search_dir() validate entry bounds
using ext4_dir_entry_len() and ext4_dirent_rec_len():
fs/ext4/dir.c:__ext4_check_dir_entry() {
else if (unlikely(rlen < ext4_dir_entry_len(de, size, fake ? NULL : dir)))
error_msg = "rec_len is too small for name_len";
}
fs/ext4/namei.c:ext4_search_dir() {
if (de->name + de->name_len <= dlimit &&
(!ext4_hash_in_dirent(dir) ||
(char *)de + ext4_dirent_rec_len(de->name_len, dir) <= dlimit) &&
ext4_match(dir, fname, de)) {
}
When ext4_has_feature_dirdata(dir->i_sb) is true and an entry in an
encrypted, casefolded directory does not have a valid EXT4_DIRENT_CFHASH
extension (for example, an un-migrated entry or a crafted entry with rec_len =
12 and name_len = 4 at the end of dir_buf in ext4_inlinedir_to_tree() or at
dlimit - 12 in ext4_search_dir()), bounds validation passes without requiring
the 8-byte hash, and the fallback to EXT4_DIRENT_HASHES(de) reads 8 bytes past
the end of the kmalloc(inline_size) buffer or directory block buffer.
Likewise, ext4_dir_entry_len() omits the 8-byte legacy hash for un-migrated
entries lacking EXT4_DIRENT_CFHASH, causing ext4_insert_dentry_data(),
dx_move_dirents(), and dx_pack_dirents() to overwrite their hashes.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029122413.230409-1-ablagodarenko@thelustrecollective.com?part=8
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v13 09/11] ext4: add dirdata LUFID support for directory entry rename
2026-10-29 12:23 [PATCH v13 00/11] Data in direntry (dirdata) feature Artem Blagodarenko
` (7 preceding siblings ...)
2026-10-29 12:24 ` [PATCH v13 08/11] ext4: dirdata feature Artem Blagodarenko
@ 2026-10-29 12:24 ` Artem Blagodarenko
2026-10-29 12:47 ` sashiko-bot
2026-10-29 12:24 ` [PATCH v13 10/11] ext4: add dirdata set/get helpers Artem Blagodarenko
2026-10-29 12:24 ` [PATCH v13 11/11] ext4: Add EXT4_IOC_SET_LUFID ioctl for setting LUFID on directory entries Artem Blagodarenko
10 siblings, 1 reply; 23+ messages in thread
From: Artem Blagodarenko @ 2026-10-29 12:24 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, jack, adilger.kernel, ablagodarenko, Artem Blagodarenko
ext4_setent() updates the inode number and file_type in an existing
directory entry but did not handle the dirdata extension payload.
Extend it with a src_fid parameter so that the source LUFID travels
with the inode through rename.
When src_fid is non-NULL and the destination slot carries a LUFID of
the same on-disk size, the source payload is copied into the destination
before the file_type merge. When src_fid is NULL (source has no LUFID),
EXT4_DIRENT_LUFID is cleared from the destination entry.
CFHASH bytes are left in place: CFHASH is a property of the destination
filename, not the inode, and it always precedes CFHASH in the extension
layout so its offset is unaffected by the LUFID copy.
Both ext4_rename() and ext4_cross_rename() snapshot the source LUFID
into a stack buffer before any setent() call, so that a cross-directory
swap reads both sides before either is overwritten.
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
---
fs/ext4/namei.c | 200 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 185 insertions(+), 15 deletions(-)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 30bbf6a56bf6..433bee757ea4 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -3574,11 +3574,6 @@ int __ext4_link(struct inode *dir, struct inode *inode,
if (IS_DIRSYNC(dir))
ext4_handle_sync(handle);
- /* Fast-commit does not record dirdata in its journal format; force a
- * full journal commit so the LUFID survives crash+replay. */
- if (ext4_dentry_get_fid(dir->i_sb, dentry ? dentry->d_fsdata : NULL))
- ext4_fc_mark_ineligible(dir->i_sb, EXT4_FC_REASON_DIRDATA, handle);
-
inode_set_ctime_current(inode);
ext4_inc_count(inode);
@@ -3752,7 +3747,8 @@ static int ext4_rename_dir_finish(handle_t *handle, struct ext4_renament *ent,
}
static int ext4_setent(handle_t *handle, struct ext4_renament *ent,
- unsigned ino, unsigned file_type)
+ unsigned ino, unsigned file_type,
+ const struct ext4_dirent_fid *src_fid)
{
int retval, retval2;
@@ -3762,8 +3758,57 @@ static int ext4_setent(handle_t *handle, struct ext4_renament *ent,
if (retval)
return retval;
ent->de->inode = cpu_to_le32(ino);
- if (ext4_has_feature_filetype(ent->dir->i_sb))
- ent->de->file_type = file_type;
+ if (ext4_has_feature_filetype(ent->dir->i_sb)) {
+ /* Copy the source LUFID payload into the destination slot when
+ * both carry a LUFID of the same on-disk size. LUFID is
+ * inode-specific and must travel with the inode through rename.
+ * CFHASH bytes are left in-place: CFHASH is a function of the
+ * destination filename, and its bytes sit at the correct offset
+ * because LUFID always precedes CFHASH in the extension layout.
+ * When src_fid is NULL, the LUFID flag is synced to the LUFID
+ * bit already present in file_type: callers that clear LUFID
+ * (rename of a non-LUFID inode, whiteout) pass file_type with
+ * LUFID=0; ext4_resetent passes the original file_type so the
+ * flag is restored for error recovery without overwriting the
+ * LUFID bytes that are still intact on disk. */
+ if (ext4_has_feature_dirdata(ent->dir->i_sb)) {
+ if (src_fid && (ent->de->file_type & EXT4_DIRENT_LUFID)) {
+ unsigned int rec_len =
+ ext4_rec_len_from_disk(ent->de->rec_len,
+ ent->dir->i_sb->s_blocksize);
+ unsigned int ddh_off =
+ EXT4_BASE_DIR_LEN + ent->de->name_len + 1;
+ struct ext4_dirent_data_header *ddh =
+ (struct ext4_dirent_data_header *)
+ ((char *)ent->de + ddh_off);
+ unsigned int copy_len = src_fid->df_header.ddh_length;
+
+ if (ddh_off + sizeof(*ddh) > rec_len ||
+ ddh->ddh_length != copy_len ||
+ ddh_off + copy_len > rec_len) {
+ /* Cannot copy: clear the flag so the
+ * slot does not advertise a stale LUFID
+ * from the old inode. */
+ ent->de->file_type &= ~EXT4_DIRENT_LUFID;
+ } else {
+ memcpy(ddh, src_fid, copy_len);
+ }
+ } else if (!src_fid) {
+ /* Sync the LUFID flag with what file_type requests.
+ * For normal rename (source has no LUFID) and for
+ * the whiteout setent, file_type carries LUFID=0,
+ * so we clear the stale flag. For ext4_resetent
+ * (error recovery), file_type is the original
+ * file_type with LUFID=1, so we restore the flag —
+ * the LUFID bytes are still on disk untouched. */
+ ent->de->file_type =
+ (ent->de->file_type & ~EXT4_DIRENT_LUFID) |
+ (file_type & EXT4_DIRENT_LUFID);
+ }
+ }
+ ent->de->file_type = (file_type & EXT4_FT_MASK) |
+ (ent->de->file_type & ~EXT4_FT_MASK);
+ }
inode_inc_iversion(ent->dir);
inode_set_mtime_to_ts(ent->dir, inode_set_ctime_current(ent->dir));
retval = ext4_mark_inode_dirty(handle, ent->dir);
@@ -3800,7 +3845,7 @@ static void ext4_resetent(handle_t *handle, struct ext4_renament *ent,
return;
}
- ext4_setent(handle, &old, ino, file_type);
+ ext4_setent(handle, &old, ino, file_type, NULL);
brelse(old.bh);
}
@@ -3899,6 +3944,67 @@ static struct inode *ext4_whiteout_for_rename(struct mnt_idmap *idmap,
return wh;
}
+/*
+ * Stack buffer sized for 1-byte header + 4 × 16-byte FIDs, covering virtually
+ * all real-world LUFID payloads without a heap allocation. ddh_length is u8
+ * so the absolute maximum is 255 bytes; larger payloads fall back to kmalloc.
+ */
+#define EXT4_LUFID_INLINE_SIZE 65
+
+struct ext4_lufid_snap {
+ struct ext4_dirent_fid *fid; /* NULL or valid; never ERR_PTR */
+ bool on_heap;
+ u8 buf[EXT4_LUFID_INLINE_SIZE];
+};
+
+/*
+ * ext4_lufid_snapshot - copy the LUFID record from a directory entry
+ *
+ * Fills @snap with a copy of the on-disk LUFID record. For payloads up to
+ * EXT4_LUFID_INLINE_SIZE bytes the inline buffer is used; larger payloads
+ * fall back to a heap allocation. Returns 0 on success (snap->fid is NULL
+ * when no LUFID is present), or -ENOMEM when a heap allocation fails.
+ * Call ext4_lufid_snap_release() when done.
+ */
+static int ext4_lufid_snapshot(struct ext4_dir_entry_2 *de,
+ unsigned int blocksize,
+ struct ext4_lufid_snap *snap)
+{
+ unsigned int ddh_off = EXT4_BASE_DIR_LEN + de->name_len + 1;
+ unsigned int rec_len = ext4_rec_len_from_disk(de->rec_len, blocksize);
+ struct ext4_dirent_fid *disk_fid;
+ unsigned int dlen;
+
+ snap->fid = NULL;
+ snap->on_heap = false;
+
+ if (!(de->file_type & EXT4_DIRENT_LUFID))
+ return 0;
+ if (ddh_off + sizeof(disk_fid->df_header) > rec_len)
+ return 0;
+ disk_fid = (struct ext4_dirent_fid *)((char *)de + ddh_off);
+ dlen = disk_fid->df_header.ddh_length;
+ if (dlen < sizeof(disk_fid->df_header) || ddh_off + dlen > rec_len)
+ return 0;
+
+ if (dlen <= sizeof(snap->buf)) {
+ memcpy(snap->buf, disk_fid, dlen);
+ snap->fid = (struct ext4_dirent_fid *)snap->buf;
+ } else {
+ snap->fid = kmemdup(disk_fid, dlen, GFP_NOFS);
+ if (!snap->fid)
+ return -ENOMEM;
+ snap->on_heap = true;
+ }
+ return 0;
+}
+
+static void ext4_lufid_snap_release(struct ext4_lufid_snap *snap)
+{
+ if (snap->on_heap)
+ kfree(snap->fid);
+}
+
/*
* Anybody can rename anything with this: the permission checks are left to the
* higher-level routines.
@@ -3925,8 +4031,10 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir,
int force_reread;
int retval;
struct inode *whiteout = NULL;
+ struct ext4_dentry_param *edp = NULL;
int credits;
u8 old_file_type;
+ struct ext4_lufid_snap old_snap = {};
if (new.inode && new.inode->i_nlink == 0) {
EXT4_ERROR_INODE(new.inode,
@@ -4032,29 +4140,65 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir,
force_reread = (new.dir->i_ino == old.dir->i_ino &&
ext4_test_inode_flag(new.dir, EXT4_INODE_INLINE_DATA));
+ if (ext4_has_feature_dirdata(old.dir->i_sb)) {
+ /* Snapshot before whiteout's ext4_setent() clears the LUFID
+ * flag in old.de; the snapshot is also needed for the !new.bh
+ * path below where we must carry the LUFID to the new entry. */
+ retval = ext4_lufid_snapshot(old.de,
+ old.dir->i_sb->s_blocksize,
+ &old_snap);
+ if (retval)
+ goto end_rename;
+ if (!old_snap.fid)
+ old_file_type &= ~EXT4_DIRENT_LUFID;
+ }
+
+ /* Allocate the LUFID wrapper for ext4_add_entry() before any
+ * destructive changes to the old entry. An OOM failure after
+ * ext4_setent() has already compacted the extensions would trigger
+ * the lossy resetent path; pre-allocating here avoids that. */
+ if (!new.bh && old_snap.fid) {
+ unsigned fid_size = old_snap.fid->df_header.ddh_length;
+
+ edp = kmalloc(offsetof(struct ext4_dentry_param,
+ edp_dfid) + fid_size, GFP_NOFS);
+ if (!edp) {
+ retval = -ENOMEM;
+ goto end_rename;
+ }
+ edp->edp_magic = EXT4_LUFID_MAGIC;
+ memcpy(&edp->edp_dfid, old_snap.fid, fid_size);
+ }
+
if (whiteout) {
/*
* Do this before adding a new entry, so the old entry is sure
* to be still pointing to the valid old entry.
*/
retval = ext4_setent(handle, &old, whiteout->i_ino,
- EXT4_FT_CHRDEV);
+ EXT4_FT_CHRDEV, NULL);
if (retval)
goto end_rename;
retval = ext4_mark_inode_dirty(handle, whiteout);
if (unlikely(retval))
goto end_rename;
-
}
if (!new.bh) {
+ new.dentry->d_fsdata = edp;
retval = ext4_add_entry(handle, new.dentry, old.inode);
+ new.dentry->d_fsdata = NULL;
if (retval)
goto end_rename;
} else {
retval = ext4_setent(handle, &new,
- old.inode->i_ino, old_file_type);
+ old.inode->i_ino, old_file_type, old_snap.fid);
if (retval)
goto end_rename;
+ /* ext4_setent() writes the LUFID in-place without going through
+ * __ext4_add_entry(), so mark ineligible here if needed. */
+ if (old_snap.fid)
+ ext4_fc_mark_ineligible(old.dir->i_sb,
+ EXT4_FC_REASON_DIRDATA, handle);
}
if (force_reread)
force_reread = !ext4_test_inode_flag(new.dir,
@@ -4139,6 +4283,8 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir,
retval = 0;
end_rename:
+ kfree(edp);
+ ext4_lufid_snap_release(&old_snap);
if (whiteout) {
if (retval) {
ext4_resetent(handle, &old,
@@ -4175,8 +4321,9 @@ static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
.dentry = new_dentry,
.inode = d_inode(new_dentry),
};
- u8 new_file_type;
+ u8 new_file_type, old_de_file_type;
int retval;
+ struct ext4_lufid_snap old_snap = {}, new_snap = {};
if ((ext4_test_inode_flag(new_dir, EXT4_INODE_PROJINHERIT) &&
!projid_eq(EXT4_I(new_dir)->i_projid,
@@ -4255,12 +4402,33 @@ static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
goto end_rename;
}
+ if (ext4_has_feature_dirdata(old.dir->i_sb)) {
+ retval = ext4_lufid_snapshot(old.de, old.dir->i_sb->s_blocksize,
+ &old_snap);
+ if (retval)
+ goto end_rename;
+ retval = ext4_lufid_snapshot(new.de, new.dir->i_sb->s_blocksize,
+ &new_snap);
+ if (retval)
+ goto end_rename;
+ if (old_snap.fid || new_snap.fid)
+ ext4_fc_mark_ineligible(old.dir->i_sb,
+ EXT4_FC_REASON_DIRDATA, handle);
+ }
+
+ old_de_file_type = old.de->file_type;
+ if (!old_snap.fid)
+ old_de_file_type &= ~EXT4_DIRENT_LUFID;
new_file_type = new.de->file_type;
- retval = ext4_setent(handle, &new, old.inode->i_ino, old.de->file_type);
+ if (!new_snap.fid)
+ new_file_type &= ~EXT4_DIRENT_LUFID;
+ retval = ext4_setent(handle, &new, old.inode->i_ino, old_de_file_type,
+ old_snap.fid);
if (retval)
goto end_rename;
- retval = ext4_setent(handle, &old, new.inode->i_ino, new_file_type);
+ retval = ext4_setent(handle, &old, new.inode->i_ino, new_file_type,
+ new_snap.fid);
if (retval)
goto end_rename;
@@ -4293,6 +4461,8 @@ static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
retval = 0;
end_rename:
+ ext4_lufid_snap_release(&old_snap);
+ ext4_lufid_snap_release(&new_snap);
brelse(old.dir_bh);
brelse(new.dir_bh);
brelse(old.bh);
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v13 09/11] ext4: add dirdata LUFID support for directory entry rename
2026-10-29 12:24 ` [PATCH v13 09/11] ext4: add dirdata LUFID support for directory entry rename Artem Blagodarenko
@ 2026-10-29 12:47 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-10-29 12:47 UTC (permalink / raw)
To: Artem Blagodarenko, linux-ext4; +Cc: tytso
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ext4: directory corruption in ext4_resetent() after RENAME_WHITEOUT failure
- [Medium] ext4: double-fetch of ddh_length in ext4_lufid_snapshot()
--
commit 05f91dc592ccfff134532dbb86b63e7c812e5a35
Author: Artem Blagodarenko <artem.blagodarenko@gmail.com>
ext4: add dirdata LUFID support for directory entry rename
This commit extends ext4_setent(), ext4_rename(), and ext4_cross_rename() to
snapshot and preserve dirdata LUFID payloads across directory entry renames.
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 30bbf6a56bf64..433bee757ea4c 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
[ ... ]
> @@ -3762,8 +3758,57 @@ static int ext4_setent(handle_t *handle, struct ext4_renament *ent,
> if (retval)
> return retval;
> ent->de->inode = cpu_to_le32(ino);
> - if (ext4_has_feature_filetype(ent->dir->i_sb))
> - ent->de->file_type = file_type;
> + if (ext4_has_feature_filetype(ent->dir->i_sb)) {
> + /* Copy the source LUFID payload into the destination slot when
> + * both carry a LUFID of the same on-disk size. LUFID is
> + * inode-specific and must travel with the inode through rename.
> + * CFHASH bytes are left in-place: CFHASH is a function of the
> + * destination filename, and its bytes sit at the correct offset
> + * because LUFID always precedes CFHASH in the extension layout.
> + * When src_fid is NULL, the LUFID flag is synced to the LUFID
> + * bit already present in file_type: callers that clear LUFID
> + * (rename of a non-LUFID inode, whiteout) pass file_type with
> + * LUFID=0; ext4_resetent passes the original file_type so the
> + * flag is restored for error recovery without overwriting the
> + * LUFID bytes that are still intact on disk. */
> + if (ext4_has_feature_dirdata(ent->dir->i_sb)) {
> + if (src_fid && (ent->de->file_type & EXT4_DIRENT_LUFID)) {
> + unsigned int rec_len =
> + ext4_rec_len_from_disk(ent->de->rec_len,
> + ent->dir->i_sb->s_blocksize);
> + unsigned int ddh_off =
> + EXT4_BASE_DIR_LEN + ent->de->name_len + 1;
> + struct ext4_dirent_data_header *ddh =
> + (struct ext4_dirent_data_header *)
> + ((char *)ent->de + ddh_off);
> + unsigned int copy_len = src_fid->df_header.ddh_length;
> +
> + if (ddh_off + sizeof(*ddh) > rec_len ||
> + ddh->ddh_length != copy_len ||
> + ddh_off + copy_len > rec_len) {
> + /* Cannot copy: clear the flag so the
> + * slot does not advertise a stale LUFID
> + * from the old inode. */
> + ent->de->file_type &= ~EXT4_DIRENT_LUFID;
> + } else {
> + memcpy(ddh, src_fid, copy_len);
> + }
> + } else if (!src_fid) {
> + /* Sync the LUFID flag with what file_type requests.
> + * For normal rename (source has no LUFID) and for
> + * the whiteout setent, file_type carries LUFID=0,
> + * so we clear the stale flag. For ext4_resetent
> + * (error recovery), file_type is the original
> + * file_type with LUFID=1, so we restore the flag —
> + * the LUFID bytes are still on disk untouched. */
> + ent->de->file_type =
> + (ent->de->file_type & ~EXT4_DIRENT_LUFID) |
> + (file_type & EXT4_DIRENT_LUFID);
> + }
> + }
> + ent->de->file_type = (file_type & EXT4_FT_MASK) |
> + (ent->de->file_type & ~EXT4_FT_MASK);
> + }
[Severity: High]
Are the LUFID bytes in old.de guaranteed to remain untouched on disk if a
RENAME_WHITEOUT operation fails and rolls back via ext4_resetent()?
In ext4_rename(), when whiteout is non-NULL, ext4_setent() is called on old
with EXT4_FT_CHRDEV and src_fid = NULL before ext4_add_entry() runs:
if (whiteout) {
retval = ext4_setent(handle, &old, whiteout->i_ino,
EXT4_FT_CHRDEV, NULL);
if (retval)
goto end_rename;
retval = ext4_mark_inode_dirty(handle, whiteout);
if (unlikely(retval))
goto end_rename;
}
if (!new.bh) {
new.dentry->d_fsdata = edp;
retval = ext4_add_entry(handle, new.dentry, old.inode);
Because that first ext4_setent() call clears EXT4_DIRENT_LUFID in
old.de->file_type, ext4_dir_entry_len() computes the active length of old.de
without its LUFID payload.
When !new.bh and old.dir == new.dir (same-directory rename with
RENAME_WHITEOUT), ext4_add_entry() can reuse or discard those LUFID bytes:
- ext4_find_dest_de() and ext4_insert_dentry_data() treat old.de's LUFID
bytes as unused slack space, shrink old.de->rec_len to the smaller active
length (smaller than ddh_off + ddh_length), and overwrite old.de's LUFID
bytes with the new directory entry.
- During an htree block split in make_indexed_dir() or do_split(),
dx_move_dirents() and dx_pack_dirents() copy only the active length for
old.de, discarding its LUFID payload and shrinking old.de->rec_len.
- Additionally, after a later patch in this series adds
ext4_setent_compact_exts(), if old.de carried both EXT4_DIRENT_LUFID and
EXT4_DIRENT_CFHASH, the whiteout ext4_setent() call slides the CFHASH
record over the LUFID slot at ddh_off.
If ext4_add_entry() (for example, returning -ENOSPC when the directory or
filesystem is full) or a later step in ext4_rename() fails and jumps to
end_rename, ext4_lufid_snap_release(&old_snap) frees old_snap and
ext4_resetent() calls ext4_setent(handle, &old, ino, file_type, NULL) with
old_file_type:
end_rename:
kfree(edp);
ext4_lufid_snap_release(&old_snap);
if (whiteout) {
if (retval) {
ext4_resetent(handle, &old,
old.inode->i_ino, old_file_type);
Wouldn't the !src_fid branch in ext4_setent() then unconditionally set
EXT4_DIRENT_LUFID back on old.de->file_type without checking whether
old.de->rec_len still encompasses the LUFID or restoring old_snap.fid,
leaving a corrupted directory entry on disk?
[ ... ]
> @@ -3899,6 +3944,67 @@ static struct inode *ext4_whiteout_for_rename(struct mnt_idmap *idmap,
> return wh;
> }
>
> +/*
> + * Stack buffer sized for 1-byte header + 4 × 16-byte FIDs, covering virtually
> + * all real-world LUFID payloads without a heap allocation. ddh_length is u8
> + * so the absolute maximum is 255 bytes; larger payloads fall back to kmalloc.
> + */
> +#define EXT4_LUFID_INLINE_SIZE 65
> +
> +struct ext4_lufid_snap {
> + struct ext4_dirent_fid *fid; /* NULL or valid; never ERR_PTR */
> + bool on_heap;
> + u8 buf[EXT4_LUFID_INLINE_SIZE];
> +};
> +
> +/*
> + * ext4_lufid_snapshot - copy the LUFID record from a directory entry
> + *
> + * Fills @snap with a copy of the on-disk LUFID record. For payloads up to
> + * EXT4_LUFID_INLINE_SIZE bytes the inline buffer is used; larger payloads
> + * fall back to a heap allocation. Returns 0 on success (snap->fid is NULL
> + * when no LUFID is present), or -ENOMEM when a heap allocation fails.
> + * Call ext4_lufid_snap_release() when done.
> + */
> +static int ext4_lufid_snapshot(struct ext4_dir_entry_2 *de,
> + unsigned int blocksize,
> + struct ext4_lufid_snap *snap)
> +{
> + unsigned int ddh_off = EXT4_BASE_DIR_LEN + de->name_len + 1;
> + unsigned int rec_len = ext4_rec_len_from_disk(de->rec_len, blocksize);
> + struct ext4_dirent_fid *disk_fid;
> + unsigned int dlen;
> +
> + snap->fid = NULL;
> + snap->on_heap = false;
> +
> + if (!(de->file_type & EXT4_DIRENT_LUFID))
> + return 0;
> + if (ddh_off + sizeof(disk_fid->df_header) > rec_len)
> + return 0;
> + disk_fid = (struct ext4_dirent_fid *)((char *)de + ddh_off);
> + dlen = disk_fid->df_header.ddh_length;
> + if (dlen < sizeof(disk_fid->df_header) || ddh_off + dlen > rec_len)
> + return 0;
> +
> + if (dlen <= sizeof(snap->buf)) {
> + memcpy(snap->buf, disk_fid, dlen);
> + snap->fid = (struct ext4_dirent_fid *)snap->buf;
> + } else {
> + snap->fid = kmemdup(disk_fid, dlen, GFP_NOFS);
> + if (!snap->fid)
> + return -ENOMEM;
> + snap->on_heap = true;
> + }
> + return 0;
> +}
[Severity: Medium]
Can a concurrent modification to the underlying buffer_head cause a
double-fetch of ddh_length in ext4_lufid_snapshot() and an out-of-bounds read
from snap->fid?
ext4_lufid_snapshot() reads dlen from disk_fid->df_header.ddh_length in the
directory buffer, validates dlen against rec_len, and then copies dlen bytes
from disk_fid into snap->buf or a kmemdup(..., GFP_NOFS) buffer (which may
sleep in reclaim before copying). During memcpy() or kmemdup(),
disk_fid->df_header.ddh_length is fetched a second time from the buffer into
snap->fid->df_header.ddh_length.
If the buffer data is concurrently modified via the underlying block device
page cache (the same threat model guarded against with
READ_ONCE(ddh->ddh_length) in the earlier commit "ext4: add dirdata format
definitions and access helpers"), snap->fid->df_header.ddh_length can become
larger than dlen (up to 255).
When ext4_rename() or ext4_setent() later reads
old_snap.fid->df_header.ddh_length or src_fid->df_header.ddh_length:
if (!new.bh && old_snap.fid) {
unsigned fid_size = old_snap.fid->df_header.ddh_length;
edp = kmalloc(offsetof(struct ext4_dentry_param,
edp_dfid) + fid_size, GFP_NOFS);
...
memcpy(&edp->edp_dfid, old_snap.fid, fid_size);
}
and in ext4_setent():
unsigned int copy_len = src_fid->df_header.ddh_length;
...
memcpy(ddh, src_fid, copy_len);
wouldn't memcpy() read past the end of old_snap.buf or the dlen-byte
kmemdup() allocation? Should ext4_lufid_snapshot() use
READ_ONCE(disk_fid->df_header.ddh_length) and explicitly set
snap->fid->df_header.ddh_length = dlen after the copy?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029122413.230409-1-ablagodarenko@thelustrecollective.com?part=9
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v13 10/11] ext4: add dirdata set/get helpers
2026-10-29 12:23 [PATCH v13 00/11] Data in direntry (dirdata) feature Artem Blagodarenko
` (8 preceding siblings ...)
2026-10-29 12:24 ` [PATCH v13 09/11] ext4: add dirdata LUFID support for directory entry rename Artem Blagodarenko
@ 2026-10-29 12:24 ` Artem Blagodarenko
2026-10-29 12:44 ` sashiko-bot
2026-10-29 12:24 ` [PATCH v13 11/11] ext4: Add EXT4_IOC_SET_LUFID ioctl for setting LUFID on directory entries Artem Blagodarenko
10 siblings, 1 reply; 23+ messages in thread
From: Artem Blagodarenko @ 2026-10-29 12:24 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, jack, adilger.kernel, ablagodarenko, Artem Blagodarenko,
Andreas Dilger
From: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Add helpers to set and retrieve dirdata payload and hook them up at
the appropriate call sites.
Enable dirdata for casefold+encryption hashes and storing unique
128-bit file identifier in the directory entry for testing.
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
---
fs/ext4/dir.c | 15 +-
fs/ext4/ext4.h | 4 +
fs/ext4/inline.c | 22 ++-
fs/ext4/namei.c | 394 +++++++++++++++++++++++++++++++++++++++++------
4 files changed, 375 insertions(+), 60 deletions(-)
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 15816f2a5f47..12df2d2a0997 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -302,8 +302,19 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
u32 minor_hash;
if (IS_CASEFOLDED(inode)) {
- hash = EXT4_DIRENT_HASH(de);
- minor_hash = EXT4_DIRENT_MINOR_HASH(de);
+ if (ext4_has_feature_dirdata(sb) &&
+ (de->file_type &
+ EXT4_DIRENT_CFHASH)) {
+ struct dx_hash_info hi = {};
+ ext4_dirdata_get(de, inode,
+ sb->s_blocksize,
+ NULL, &hi);
+ hash = hi.hash;
+ minor_hash = hi.minor_hash;
+ } else {
+ hash = EXT4_DIRENT_HASH(de);
+ minor_hash = EXT4_DIRENT_MINOR_HASH(de);
+ }
} else {
hash = 0;
minor_hash = 0;
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index aea75a320ceb..086a8a0d340f 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3980,6 +3980,10 @@ extern int __ext4_unlink(struct inode *dir, const struct qstr *d_name,
struct inode *inode, struct dentry *dentry);
extern int __ext4_link(struct inode *dir, struct inode *inode,
const struct qstr *d_name, struct dentry *dentry);
+extern unsigned char ext4_dirdata_get(struct ext4_dir_entry_2 *de,
+ struct inode *dir, int buf_size,
+ struct ext4_dirent_fid *lufid,
+ struct dx_hash_info *hinfo);
#define S_SHIFT 12
static const unsigned char ext4_type_by_mode[(S_IFMT >> S_SHIFT) + 1] = {
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index d3dda1d65d06..700a0282278b 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1364,14 +1364,20 @@ int ext4_inlinedir_to_tree(struct file *dir_file,
pos += de_len;
}
- if (ext4_hash_in_dirent(dir)) {
- hinfo->hash = EXT4_DIRENT_HASH(de);
- hinfo->minor_hash = EXT4_DIRENT_MINOR_HASH(de);
- } else {
- err = ext4fs_dirhash(dir, de->name, de->name_len, hinfo);
- if (err) {
- ret = err;
- goto out;
+ if (!(ext4_dirdata_get(de, dir, inline_size, NULL, hinfo) &
+ EXT4_DIRENT_CFHASH)) {
+ if (ext4_hash_in_dirent(dir)) {
+ /* Un-migrated entry: hash at legacy fixed
+ * offset. */
+ hinfo->hash = EXT4_DIRENT_HASH(de);
+ hinfo->minor_hash = EXT4_DIRENT_MINOR_HASH(de);
+ } else {
+ err = ext4fs_dirhash(dir, de->name,
+ de->name_len, hinfo);
+ if (err) {
+ ret = err;
+ goto out;
+ }
}
}
if ((hinfo->hash < start_hash) ||
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 433bee757ea4..5a118320eef9 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1142,22 +1142,34 @@ static int htree_dirblock_to_tree(struct file *dir_file,
/* silently ignore the rest of the block */
break;
}
- if (ext4_hash_in_dirent(dir)) {
- if (de->name_len && de->inode) {
- hinfo->hash = EXT4_DIRENT_HASH(de);
- hinfo->minor_hash = EXT4_DIRENT_MINOR_HASH(de);
- } else {
- hinfo->hash = 0;
- hinfo->minor_hash = 0;
- }
+ if (de->name_len && de->inode) {
+ /* check for saved hash first, or generate it from name */
+ if (!(ext4_dirdata_get(de, dir, dir->i_sb->s_blocksize,
+ NULL, hinfo) &
+ EXT4_DIRENT_CFHASH)) {
+ if (ext4_hash_in_dirent(dir)) {
+ /* Un-migrated entry: hash is at the
+ * legacy fixed offset, not in a CFHASH
+ * extension. Read it directly rather
+ * than hashing the encrypted name. */
+ hinfo->hash = EXT4_DIRENT_HASH(de);
+ hinfo->minor_hash =
+ EXT4_DIRENT_MINOR_HASH(de);
+ } else {
+ err = ext4fs_dirhash(dir, de->name,
+ de->name_len,
+ hinfo);
+ if (err < 0) {
+ count = err;
+ goto errout;
+ }
+ }
+ }
} else {
- err = ext4fs_dirhash(dir, de->name,
- de->name_len, hinfo);
- if (err < 0) {
- count = err;
- goto errout;
- }
+ hinfo->hash = 0;
+ hinfo->minor_hash = 0;
}
+
if ((hinfo->hash < start_hash) ||
((hinfo->hash == start_hash) &&
(hinfo->minor_hash < start_minor_hash)))
@@ -1337,6 +1349,202 @@ static inline int search_dirblock(struct buffer_head *bh,
* Directory block splitting, compacting
*/
+/*
+ * ext4_dirdata_get() - Read dirdata fields from a directory entry.
+ * @de: directory entry
+ * @dir: directory inode (used for fscrypt+casefold hash fallback)
+ * @dfid: if non-NULL and EXT4_DIRENT_LUFID is set, LUFID data is copied
+ * here
+ * @hinfo: if non-NULL, receives the casefold hash and minor hash
+ *
+ * Reads any dirdata stored in @de. If the dirdata feature is not enabled,
+ * falls back to reading the hash stored inline after the filename (for
+ * compatibility with the older casefold+fscrypt format).
+ *
+ * Returns a bitmask of EXT4_DIRENT_* flags indicating which fields were read.
+ *
+ * Compatibility note: enabling EXT4_FEATURE_INCOMPAT_DIRDATA on a filesystem
+ * that already has casefolded+encrypted directories is NOT safe without a
+ * prior migration pass. Before dirdata, the casefold+fscrypt hash was stored
+ * as a raw 8 bytes immediately after the filename with no flag in file_type.
+ * After dirdata is enabled, this function expects the hash to be present only
+ * when EXT4_DIRENT_CFHASH (0x40) is set in file_type; existing entries that
+ * carry the raw hash are silently misread as having no hash at all, breaking
+ * directory lookups. e2fsck must be run to convert all affected entries to
+ * the EXT4_DIRENT_CFHASH extension format before the feature flag is set with
+ * tune2fs. Detection heuristic: in a casefold+encrypted directory, an entry
+ * with rec_len >= round_up(name_len, 4) + 8 and no EXT4_DIRENT_CFHASH bit
+ * carries a raw pre-dirdata hash that must be migrated.
+ */
+unsigned char ext4_dirdata_get(struct ext4_dir_entry_2 *de, struct inode *dir,
+ int buf_size,
+ struct ext4_dirent_fid *dfid,
+ struct dx_hash_info *hinfo)
+{
+ unsigned char ret = 0;
+ unsigned int data_offset = de->name_len + 1;
+ unsigned int rec_len = ext4_rec_len_from_disk(de->rec_len, buf_size);
+
+ /* data_offset is relative to de->name, which itself starts
+ * EXT4_BASE_DIR_LEN bytes into the entry -- rec_len is relative to
+ * the start of the entry, so add the header size before comparing,
+ * or this lets reads run EXT4_BASE_DIR_LEN bytes past the entry. */
+ if (EXT4_BASE_DIR_LEN + data_offset > rec_len)
+ return ret;
+
+ /* compatibility: hash stored inline after filename (no dirdata) */
+ if (hinfo && !ext4_has_feature_dirdata(dir->i_sb) &&
+ ext4_hash_in_dirent(dir)) {
+ hinfo->hash = EXT4_DIRENT_HASH(de);
+ hinfo->minor_hash = EXT4_DIRENT_MINOR_HASH(de);
+ ret |= EXT4_DIRENT_CFHASH;
+ return ret;
+ }
+
+ /* EXT4_DIRENT_* bits are only meaningful when the feature is enabled */
+ if (!ext4_has_feature_dirdata(dir->i_sb))
+ return ret;
+
+ if (de->file_type & EXT4_DIRENT_LUFID) {
+ struct ext4_dirent_fid *disk_fid =
+ (struct ext4_dirent_fid *)((char *)de +
+ EXT4_BASE_DIR_LEN + data_offset);
+ unsigned int dlen;
+ /* struct ext4_fid df_fid[] does not provide the array size.
+ * First, verify that the header lies within the valid area, then
+ * verify that the entire record fits within it. */
+ if (EXT4_BASE_DIR_LEN + data_offset +
+ sizeof(disk_fid->df_header) > rec_len)
+ return ret;
+
+ dlen = disk_fid->df_header.ddh_length;
+ if (dlen == 0 ||
+ EXT4_BASE_DIR_LEN + data_offset + dlen > rec_len)
+ return ret;
+
+ if (dfid) {
+ memcpy(dfid, disk_fid, dlen);
+ ret |= EXT4_DIRENT_LUFID;
+ }
+ data_offset += dlen;
+ }
+
+ /* Skip INO64 for now*/
+ if (de->file_type & EXT4_DIRENT_INO64) {
+ struct ext4_dirent_data_header *ddh =
+ (struct ext4_dirent_data_header *)((char *)de + EXT4_BASE_DIR_LEN + data_offset);
+ unsigned int dlen;
+
+ if (EXT4_BASE_DIR_LEN + data_offset + sizeof(*ddh) > rec_len)
+ return ret;
+
+ dlen = ddh->ddh_length;
+ if (dlen < sizeof(*ddh) ||
+ EXT4_BASE_DIR_LEN + data_offset + dlen > rec_len)
+ return ret;
+
+ data_offset += dlen;
+ }
+
+ if (!hinfo)
+ return ret;
+
+ if (de->file_type & EXT4_DIRENT_CFHASH) {
+ struct ext4_dirent_hash *dh =
+ (struct ext4_dirent_hash *)((char *)de + EXT4_BASE_DIR_LEN + data_offset);
+ unsigned int dlen;
+
+ if (EXT4_BASE_DIR_LEN + data_offset + sizeof(*dh) > rec_len)
+ return ret;
+ dlen = dh->dh_header.ddh_length;
+ if (dlen < sizeof(*dh) ||
+ EXT4_BASE_DIR_LEN + data_offset + dlen > rec_len)
+ return ret;
+
+ hinfo->hash = le32_to_cpu(dh->dh_hash.hash);
+ hinfo->minor_hash = le32_to_cpu(dh->dh_hash.minor_hash);
+ ret |= EXT4_DIRENT_CFHASH;
+ }
+
+ return ret;
+}
+
+/*
+ * ext4_dirdata_set() - Write dirdata fields into a directory entry.
+ * @de: directory entry (name must already be set)
+ * @dir: directory inode
+ * @data: LUFID data to store (or NULL)
+ * @fname: filename info carrying the casefold hash
+ *
+ * Writes any required dirdata into @de after the filename. If the dirdata
+ * feature is not enabled, falls back to writing the hash inline after the
+ * filename (for compatibility with the older casefold+fscrypt format).
+ *
+ * See ext4_dirdata_get() for the compatibility constraint: enabling the
+ * dirdata feature on a filesystem with existing casefolded+encrypted
+ * directories requires an e2fsck migration pass before tune2fs sets the
+ * EXT4_FEATURE_INCOMPAT_DIRDATA superblock flag.
+ */
+static void ext4_dirdata_set(struct ext4_dir_entry_2 *de, struct inode *dir,
+ struct ext4_dirent_fid *dfid,
+ struct ext4_filename *fname)
+{
+ struct dx_hash_info *hinfo = &fname->hinfo;
+ unsigned int data_offset = de->name_len + 1;
+ unsigned int rec_len = ext4_rec_len_from_disk(de->rec_len,
+ dir->i_sb->s_blocksize);
+
+ /* Clear the gap byte between the filename and the first dirdata
+ * extension to avoid leaking stale memory to disk. Use pointer
+ * arithmetic rather than de->name[name_len] to stay within the
+ * declared name[] array bounds under FORTIFY_SOURCE. Only write it
+ * when there is actually room (entries that exactly fill their slot
+ * have rec_len == EXT4_BASE_DIR_LEN + name_len with no gap). */
+ if (EXT4_BASE_DIR_LEN + data_offset <= rec_len)
+ *((char *)de + EXT4_BASE_DIR_LEN + de->name_len) = 0;
+
+ if (dfid) {
+ unsigned int dlen = dfid->df_header.ddh_length;
+
+ if (EXT4_BASE_DIR_LEN + data_offset + dlen > rec_len) {
+ EXT4_ERROR_INODE(dir, "Can not insert FID");
+ return;
+ }
+
+ memcpy((char *)de + EXT4_BASE_DIR_LEN + data_offset, dfid, dlen);
+ de->file_type |= EXT4_DIRENT_LUFID;
+ data_offset += dlen;
+ }
+
+ if (ext4_hash_in_dirent(dir)) {
+ if (ext4_has_feature_dirdata(dir->i_sb)) {
+ struct ext4_dirent_hash *dh =
+ (struct ext4_dirent_hash *)((char *)de + EXT4_BASE_DIR_LEN + data_offset);
+
+ if (EXT4_BASE_DIR_LEN + data_offset + sizeof(*dh) > rec_len) {
+ EXT4_ERROR_INODE(dir, "Can not insert dhash dirdata");
+ return;
+ }
+
+ dh->dh_header.ddh_length = sizeof(*dh);
+ dh->dh_hash.hash = cpu_to_le32(hinfo->hash);
+ dh->dh_hash.minor_hash = cpu_to_le32(hinfo->minor_hash);
+ de->file_type |= EXT4_DIRENT_CFHASH;
+ } else {
+ /* Compatibility: store hash inline after filename */
+ if (EXT4_BASE_DIR_LEN + data_offset +
+ sizeof(struct ext4_dir_entry_hash) > rec_len) {
+ EXT4_ERROR_INODE(dir, "Can not insert dhash");
+ return;
+ }
+
+ EXT4_DIRENT_HASHES(de)->hash = cpu_to_le32(hinfo->hash);
+ EXT4_DIRENT_HASHES(de)->minor_hash =
+ cpu_to_le32(hinfo->minor_hash);
+ }
+ }
+}
+
/*
* Create map of hash values, offsets, and sizes, stored at end of block.
* Returns number of entries mapped.
@@ -1360,13 +1568,21 @@ static int dx_make_map(struct inode *dir, struct buffer_head *bh,
((char *)de) - base))
return -EFSCORRUPTED;
if (de->name_len && de->inode) {
- if (ext4_hash_in_dirent(dir))
- h.hash = EXT4_DIRENT_HASH(de);
- else {
- int err = ext4fs_dirhash(dir, de->name,
- de->name_len, &h);
- if (err < 0)
- return err;
+ if (!(ext4_dirdata_get(de, dir, dir->i_sb->s_blocksize,
+ NULL, &h) &
+ EXT4_DIRENT_CFHASH)) {
+ if (ext4_hash_in_dirent(dir)) {
+ /* Un-migrated entry: hash at legacy
+ * fixed offset. */
+ h.hash = EXT4_DIRENT_HASH(de);
+ h.minor_hash =
+ EXT4_DIRENT_MINOR_HASH(de);
+ } else {
+ int err = ext4fs_dirhash(dir, de->name,
+ de->name_len, &h);
+ if (err < 0)
+ return err;
+ }
}
map_tail--;
map_tail->hash = h.hash;
@@ -1467,8 +1683,9 @@ int ext4_fname_setup_ci_filename(struct inode *dir, const struct qstr *iname,
* Return: %true if the directory entry matches, otherwise %false.
*/
static bool ext4_match(struct inode *parent,
- const struct ext4_filename *fname,
- struct ext4_dir_entry_2 *de)
+ const struct ext4_filename *fname,
+ struct ext4_dir_entry_2 *de,
+ int buf_size)
{
struct fscrypt_name f;
@@ -1495,10 +1712,29 @@ static bool ext4_match(struct inode *parent,
* considering the calculated hash.
*/
if (sb_no_casefold_compat_fallback(parent->i_sb) &&
- IS_ENCRYPTED(parent) && fname->cf_name.name &&
- (fname->hinfo.hash != EXT4_DIRENT_HASH(de) ||
- fname->hinfo.minor_hash != EXT4_DIRENT_MINOR_HASH(de)))
- return false;
+ IS_ENCRYPTED(parent) && fname->cf_name.name) {
+ __u32 de_hash, de_minor_hash;
+
+ if (ext4_has_feature_dirdata(parent->i_sb) &&
+ (de->file_type & EXT4_DIRENT_CFHASH)) {
+ /* Hash is in a CFHASH extension at a variable
+ * offset (past any LUFID bytes). Read it via
+ * ext4_dirdata_get() to get the correct offset. */
+ struct dx_hash_info dirent_hinfo = {};
+
+ ext4_dirdata_get(de, parent,
+ buf_size,
+ NULL, &dirent_hinfo);
+ de_hash = dirent_hinfo.hash;
+ de_minor_hash = dirent_hinfo.minor_hash;
+ } else {
+ de_hash = EXT4_DIRENT_HASH(de);
+ de_minor_hash = EXT4_DIRENT_MINOR_HASH(de);
+ }
+ if (fname->hinfo.hash != de_hash ||
+ fname->hinfo.minor_hash != de_minor_hash)
+ return false;
+ }
/*
* Treat comparison errors as not a match. The
* only case where it happens is on a disk
@@ -1529,11 +1765,18 @@ int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size,
dlimit = search_buf + buf_size;
while ((char *) de < dlimit - EXT4_BASE_DIR_LEN) {
/* this code is executed quadratically often */
- /* do minimal checking `by hand' */
+ /* Decode rec_len before ext4_match() so that ext4_dirdata_get()
+ * inside ext4_match() never uses an unvalidated rec_len as a
+ * read bound. A crafted entry with rec_len > buf_size would
+ * let ext4_dirdata_get() read past the block buffer. */
+ de_len = ext4_rec_len_from_disk(de->rec_len,
+ dir->i_sb->s_blocksize);
+ if (de_len <= 0 || (char *)de + de_len > dlimit)
+ return -EFSCORRUPTED;
if (de->name + de->name_len <= dlimit &&
(!ext4_hash_in_dirent(dir) ||
(char *)de + ext4_dirent_rec_len(de->name_len, dir) <= dlimit) &&
- ext4_match(dir, fname, de)) {
+ ext4_match(dir, fname, de, buf_size)) {
/* found a match - just to be sure, do
* a full check */
if (ext4_check_dir_entry(dir, NULL, de, bh, search_buf,
@@ -1542,11 +1785,6 @@ int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size,
*res_dir = de;
return 1;
}
- /* prevent looping on a bad block */
- de_len = ext4_rec_len_from_disk(de->rec_len,
- dir->i_sb->s_blocksize);
- if (de_len <= 0)
- return -EFSCORRUPTED;
offset += de_len;
de = (struct ext4_dir_entry_2 *) ((char *) de + de_len);
}
@@ -2120,7 +2358,7 @@ int ext4_find_dest_de(struct inode *dir, struct buffer_head *bh,
if (ext4_check_dir_entry(dir, NULL, de, bh,
buf, buf_size, offset))
return -EFSCORRUPTED;
- if (ext4_match(dir, fname, de))
+ if (ext4_match(dir, fname, de, buf_size))
return -EEXIST;
nlen = ext4_dir_entry_len(de, buf_size, dir);
rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
@@ -2156,17 +2394,7 @@ void ext4_insert_dentry_data(struct inode *dir, struct inode *inode,
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));
- /* 'data' (the LUFID payload) and the CFHASH are written by
- * ext4_dirdata_set(), which is introduced in the next patch that adds
- * the dirdata set/get helpers. Until then, only the casefold hash is
- * stored at the legacy fixed offset for encrypted+casefolded dirs. */
- if (ext4_hash_in_dirent(dir)) {
- struct dx_hash_info *hinfo = &fname->hinfo;
-
- EXT4_DIRENT_HASHES(de)->hash = cpu_to_le32(hinfo->hash);
- EXT4_DIRENT_HASHES(de)->minor_hash =
- cpu_to_le32(hinfo->minor_hash);
- }
+ ext4_dirdata_set(de, dir, data, fname);
}
/*
@@ -3746,6 +3974,43 @@ static int ext4_rename_dir_finish(handle_t *handle, struct ext4_renament *ent,
return 0;
}
+/*
+ * When LUFID is cleared from an entry that also carries CFHASH, the CFHASH
+ * bytes are stranded: LUFID precedes CFHASH in the layout, so removing LUFID
+ * shifts the expected CFHASH offset. Slide CFHASH into the vacated slot so
+ * the parser finds it at the right position. If the headers are malformed,
+ * clear CFHASH instead of risking a corrupt read.
+ */
+static void ext4_setent_compact_exts(struct ext4_renament *ent,
+ struct ext4_dirent_data_header *lddh,
+ unsigned int lufid_off,
+ unsigned int rec_len)
+{
+ struct ext4_dirent_data_header *cddh;
+ unsigned int coff;
+
+ if (!(ent->de->file_type & EXT4_DIRENT_CFHASH))
+ return;
+ if (lufid_off + sizeof(*lddh) > rec_len ||
+ !lddh->ddh_length ||
+ lufid_off + lddh->ddh_length > rec_len) {
+ ent->de->file_type &= ~EXT4_DIRENT_CFHASH;
+ return;
+ }
+
+ coff = lufid_off + lddh->ddh_length;
+ cddh = (struct ext4_dirent_data_header *)((char *)ent->de + coff);
+
+ if (coff + sizeof(*cddh) > rec_len ||
+ !cddh->ddh_length ||
+ coff + cddh->ddh_length > rec_len) {
+ ent->de->file_type &= ~EXT4_DIRENT_CFHASH;
+ return;
+ }
+
+ memmove(lddh, cddh, cddh->ddh_length);
+}
+
static int ext4_setent(handle_t *handle, struct ext4_renament *ent,
unsigned ino, unsigned file_type,
const struct ext4_dirent_fid *src_fid)
@@ -3786,13 +4051,25 @@ static int ext4_setent(handle_t *handle, struct ext4_renament *ent,
if (ddh_off + sizeof(*ddh) > rec_len ||
ddh->ddh_length != copy_len ||
ddh_off + copy_len > rec_len) {
- /* Cannot copy: clear the flag so the
- * slot does not advertise a stale LUFID
- * from the old inode. */
+ /* Cannot copy in-place (size mismatch).
+ * If CFHASH is also present, slide it
+ * into the LUFID slot so the parser
+ * finds it at the correct offset after
+ * the LUFID flag is cleared. */
+ ext4_setent_compact_exts(ent, ddh,
+ ddh_off, rec_len);
ent->de->file_type &= ~EXT4_DIRENT_LUFID;
} else {
memcpy(ddh, src_fid, copy_len);
}
+ } else if (src_fid) {
+ /* Destination has no LUFID slot; cannot
+ * propagate in-place. Clear the flag from
+ * the on-disk entry directly — clearing only
+ * the local file_type variable has no effect
+ * because the final assignment takes the high
+ * bits from ent->de->file_type, not file_type. */
+ ent->de->file_type &= ~EXT4_DIRENT_LUFID;
} else if (!src_fid) {
/* Sync the LUFID flag with what file_type requests.
* For normal rename (source has no LUFID) and for
@@ -3800,7 +4077,24 @@ static int ext4_setent(handle_t *handle, struct ext4_renament *ent,
* so we clear the stale flag. For ext4_resetent
* (error recovery), file_type is the original
* file_type with LUFID=1, so we restore the flag —
- * the LUFID bytes are still on disk untouched. */
+ * the LUFID bytes are still on disk untouched.
+ * When clearing LUFID and CFHASH is present, slide
+ * CFHASH bytes into the LUFID slot first so the
+ * parser finds CFHASH at the expected offset. */
+ if (!(file_type & EXT4_DIRENT_LUFID) &&
+ (ent->de->file_type & EXT4_DIRENT_LUFID)) {
+ unsigned int loff =
+ EXT4_BASE_DIR_LEN +
+ ent->de->name_len + 1;
+ unsigned int rlen =
+ ext4_rec_len_from_disk(
+ ent->de->rec_len,
+ ent->dir->i_sb->s_blocksize);
+ ext4_setent_compact_exts(ent,
+ (struct ext4_dirent_data_header *)
+ ((char *)ent->de + loff),
+ loff, rlen);
+ }
ent->de->file_type =
(ent->de->file_type & ~EXT4_DIRENT_LUFID) |
(file_type & EXT4_DIRENT_LUFID);
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v13 10/11] ext4: add dirdata set/get helpers
2026-10-29 12:24 ` [PATCH v13 10/11] ext4: add dirdata set/get helpers Artem Blagodarenko
@ 2026-10-29 12:44 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-10-29 12:44 UTC (permalink / raw)
To: Artem Blagodarenko, linux-ext4; +Cc: tytso
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ext4: out-of-bounds read in EXT4_DIRENT_HASH() fallback paths
- [High] ext4: LUFID and CFHASH corruption in ext4_setent() on whiteout rollback
--
commit 539752e69c673cf26e5b8cfb8fcd2a83e0a00a76
Author: Artem Blagodarenko <artem.blagodarenko@gmail.com>
ext4: add dirdata set/get helpers
This commit adds ext4_dirdata_get() and ext4_dirdata_set() helpers to read and
write directory entry extension payloads, and wires them into directory lookup,
readdir, htree indexing, insertion, and rename paths for LUFID and
casefold+encryption hashes.
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index 15816f2a5f472..12df2d2a09978 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -302,8 +302,19 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
> u32 minor_hash;
>
> if (IS_CASEFOLDED(inode)) {
> - hash = EXT4_DIRENT_HASH(de);
> - minor_hash = EXT4_DIRENT_MINOR_HASH(de);
> + if (ext4_has_feature_dirdata(sb) &&
> + (de->file_type &
> + EXT4_DIRENT_CFHASH)) {
> + struct dx_hash_info hi = {};
> + ext4_dirdata_get(de, inode,
> + sb->s_blocksize,
> + NULL, &hi);
> + hash = hi.hash;
> + minor_hash = hi.minor_hash;
> + } else {
> + hash = EXT4_DIRENT_HASH(de);
> + minor_hash = EXT4_DIRENT_MINOR_HASH(de);
> + }
> } else {
> hash = 0;
> minor_hash = 0;
[ ... ]
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index d3dda1d65d067..700a0282278b0 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -1364,14 +1364,20 @@ int ext4_inlinedir_to_tree(struct file *dir_file,
> pos += de_len;
> }
>
> - if (ext4_hash_in_dirent(dir)) {
> - hinfo->hash = EXT4_DIRENT_HASH(de);
> - hinfo->minor_hash = EXT4_DIRENT_MINOR_HASH(de);
> - } else {
> - err = ext4fs_dirhash(dir, de->name, de->name_len, hinfo);
> - if (err) {
> - ret = err;
> - goto out;
> + if (!(ext4_dirdata_get(de, dir, inline_size, NULL, hinfo) &
> + EXT4_DIRENT_CFHASH)) {
> + if (ext4_hash_in_dirent(dir)) {
> + /* Un-migrated entry: hash at legacy fixed
> + * offset. */
> + hinfo->hash = EXT4_DIRENT_HASH(de);
> + hinfo->minor_hash = EXT4_DIRENT_MINOR_HASH(de);
> + } else {
> + err = ext4fs_dirhash(dir, de->name,
> + de->name_len, hinfo);
> + if (err) {
> + ret = err;
> + goto out;
> + }
> }
> }
[ ... ]
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 433bee757ea4c..5a118320eef9b 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1142,22 +1142,34 @@ static int htree_dirblock_to_tree(struct file *dir_file,
> /* silently ignore the rest of the block */
> break;
> }
> - if (ext4_hash_in_dirent(dir)) {
> - if (de->name_len && de->inode) {
> - hinfo->hash = EXT4_DIRENT_HASH(de);
> - hinfo->minor_hash = EXT4_DIRENT_MINOR_HASH(de);
> - } else {
> - hinfo->hash = 0;
> - hinfo->minor_hash = 0;
> - }
> + if (de->name_len && de->inode) {
> + /* check for saved hash first, or generate it from name */
> + if (!(ext4_dirdata_get(de, dir, dir->i_sb->s_blocksize,
> + NULL, hinfo) &
> + EXT4_DIRENT_CFHASH)) {
> + if (ext4_hash_in_dirent(dir)) {
> + /* Un-migrated entry: hash is at the
> + * legacy fixed offset, not in a CFHASH
> + * extension. Read it directly rather
> + * than hashing the encrypted name. */
> + hinfo->hash = EXT4_DIRENT_HASH(de);
> + hinfo->minor_hash =
> + EXT4_DIRENT_MINOR_HASH(de);
> + } else {
[ ... ]
> +unsigned char ext4_dirdata_get(struct ext4_dir_entry_2 *de, struct inode *dir,
> + int buf_size,
> + struct ext4_dirent_fid *dfid,
> + struct dx_hash_info *hinfo)
> +{
> + unsigned char ret = 0;
> + unsigned int data_offset = de->name_len + 1;
> + unsigned int rec_len = ext4_rec_len_from_disk(de->rec_len, buf_size);
> +
> + /* data_offset is relative to de->name, which itself starts
> + * EXT4_BASE_DIR_LEN bytes into the entry -- rec_len is relative to
> + * the start of the entry, so add the header size before comparing,
> + * or this lets reads run EXT4_BASE_DIR_LEN bytes past the entry. */
> + if (EXT4_BASE_DIR_LEN + data_offset > rec_len)
> + return ret;
> +
> + /* compatibility: hash stored inline after filename (no dirdata) */
> + if (hinfo && !ext4_has_feature_dirdata(dir->i_sb) &&
> + ext4_hash_in_dirent(dir)) {
> + hinfo->hash = EXT4_DIRENT_HASH(de);
> + hinfo->minor_hash = EXT4_DIRENT_MINOR_HASH(de);
> + ret |= EXT4_DIRENT_CFHASH;
> + return ret;
> + }
[ ... ]
> @@ -1360,13 +1568,21 @@ static int dx_make_map(struct inode *dir, struct buffer_head *bh,
> ((char *)de) - base))
> return -EFSCORRUPTED;
> if (de->name_len && de->inode) {
> - if (ext4_hash_in_dirent(dir))
> - h.hash = EXT4_DIRENT_HASH(de);
> - else {
> - int err = ext4fs_dirhash(dir, de->name,
> - de->name_len, &h);
> - if (err < 0)
> - return err;
> + if (!(ext4_dirdata_get(de, dir, dir->i_sb->s_blocksize,
> + NULL, &h) &
> + EXT4_DIRENT_CFHASH)) {
> + if (ext4_hash_in_dirent(dir)) {
> + /* Un-migrated entry: hash at legacy
> + * fixed offset. */
> + h.hash = EXT4_DIRENT_HASH(de);
> + h.minor_hash =
> + EXT4_DIRENT_MINOR_HASH(de);
> + } else {
[ ... ]
> @@ -1495,10 +1712,29 @@ static bool ext4_match(struct inode *parent,
> * considering the calculated hash.
> */
> if (sb_no_casefold_compat_fallback(parent->i_sb) &&
> - IS_ENCRYPTED(parent) && fname->cf_name.name &&
> - (fname->hinfo.hash != EXT4_DIRENT_HASH(de) ||
> - fname->hinfo.minor_hash != EXT4_DIRENT_MINOR_HASH(de)))
> - return false;
> + IS_ENCRYPTED(parent) && fname->cf_name.name) {
> + __u32 de_hash, de_minor_hash;
> +
> + if (ext4_has_feature_dirdata(parent->i_sb) &&
> + (de->file_type & EXT4_DIRENT_CFHASH)) {
> + /* Hash is in a CFHASH extension at a variable
> + * offset (past any LUFID bytes). Read it via
> + * ext4_dirdata_get() to get the correct offset. */
> + struct dx_hash_info dirent_hinfo = {};
> +
> + ext4_dirdata_get(de, parent,
> + buf_size,
> + NULL, &dirent_hinfo);
> + de_hash = dirent_hinfo.hash;
> + de_minor_hash = dirent_hinfo.minor_hash;
> + } else {
> + de_hash = EXT4_DIRENT_HASH(de);
> + de_minor_hash = EXT4_DIRENT_MINOR_HASH(de);
> + }
> + if (fname->hinfo.hash != de_hash ||
> + fname->hinfo.minor_hash != de_minor_hash)
> + return false;
> + }
> /*
> * Treat comparison errors as not a match. The
> * only case where it happens is on a disk
> @@ -1529,11 +1765,18 @@ int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size,
> dlimit = search_buf + buf_size;
> while ((char *) de < dlimit - EXT4_BASE_DIR_LEN) {
> /* this code is executed quadratically often */
> - /* do minimal checking `by hand' */
> + /* Decode rec_len before ext4_match() so that ext4_dirdata_get()
> + * inside ext4_match() never uses an unvalidated rec_len as a
> + * read bound. A crafted entry with rec_len > buf_size would
> + * let ext4_dirdata_get() read past the block buffer. */
> + de_len = ext4_rec_len_from_disk(de->rec_len,
> + dir->i_sb->s_blocksize);
> + if (de_len <= 0 || (char *)de + de_len > dlimit)
> + return -EFSCORRUPTED;
> if (de->name + de->name_len <= dlimit &&
> (!ext4_hash_in_dirent(dir) ||
> (char *)de + ext4_dirent_rec_len(de->name_len, dir) <= dlimit) &&
> - ext4_match(dir, fname, de)) {
> + ext4_match(dir, fname, de, buf_size)) {
[Severity: High]
Can the EXT4_DIRENT_HASH(de) and EXT4_DIRENT_MINOR_HASH(de) fallback paths
read 8 bytes past the end of bh->b_data or dir_buf?
In fs/ext4/ext4.h:ext4_dirent_rec_len(), when EXT4_FEATURE_INCOMPAT_DIRDATA
is enabled on a casefolded and encrypted directory, sizeof(struct
ext4_dir_entry_hash) is not added to rec_len:
if (dir && ext4_hash_in_dirent(dir) &&
!ext4_has_feature_dirdata(dir->i_sb))
rec_len += sizeof(struct ext4_dir_entry_hash);
return (rec_len & ~EXT4_DIR_ROUND);
Because of this, both __ext4_check_dir_entry() and the pre-check in
fs/ext4/namei.c:ext4_search_dir() accept a trailing entry without
EXT4_DIRENT_CFHASH (or with a malformed CFHASH header where
ext4_dirent_get_data_len() and ext4_dirdata_get() return 0) whose rec_len is
((de->name_len + 8 + EXT4_DIR_ROUND) & ~EXT4_DIR_ROUND) right at the end of
the buffer.
However, EXT4_DIRENT_HASHES(de) starts at offset ((8 + de->name_len +
EXT4_DIR_ROUND) & ~EXT4_DIR_ROUND) and reads 8 bytes from that offset. When
fs/ext4/dir.c:ext4_readdir(), fs/ext4/inline.c:ext4_inlinedir_to_tree(),
fs/ext4/namei.c:htree_dirblock_to_tree(), fs/ext4/namei.c:dx_make_map(), or
fs/ext4/namei.c:ext4_match() falls back to EXT4_DIRENT_HASH(de) and
EXT4_DIRENT_MINOR_HASH(de) without checking that those 8 bytes fit within
rec_len, doesn't that read 8 bytes past de + rec_len and past the end of
bh->b_data or the kmalloc(inline_size, GFP_NOFS) buffer dir_buf?
Similarly, in fs/ext4/namei.c:ext4_dirdata_get(), the non-dirdata
compatibility branch only checks:
if (EXT4_BASE_DIR_LEN + data_offset > rec_len)
return ret;
which verifies 8 + de->name_len + 1 <= rec_len before reading 8 bytes at
((8 + de->name_len + EXT4_DIR_ROUND) & ~EXT4_DIR_ROUND). And in
fs/ext4/inline.c:ext4_inlinedir_to_tree(), the synthesized "." and ".."
entries use an uninitialized stack variable fake with rec_len == 12 and no
EXT4_DIRENT_CFHASH bit, causing both ext4_dirdata_get() and the fallback to
read 8 bytes of uninitialized kernel stack from fake.name[4..11] and pass
them to ext4_htree_store_dirent().
Should all legacy EXT4_DIRENT_HASH(de) reads verify that
((EXT4_BASE_DIR_LEN + de->name_len + EXT4_DIR_ROUND) & ~EXT4_DIR_ROUND) +
sizeof(struct ext4_dir_entry_hash) <= rec_len first?
[ ... ]
> @@ -3746,6 +3974,43 @@ static int ext4_rename_dir_finish(handle_t *handle, struct ext4_renament *ent,
> return 0;
> }
>
> +/*
> + * When LUFID is cleared from an entry that also carries CFHASH, the CFHASH
> + * bytes are stranded: LUFID precedes CFHASH in the layout, so removing LUFID
> + * shifts the expected CFHASH offset. Slide CFHASH into the vacated slot so
> + * the parser finds it at the right position. If the headers are malformed,
> + * clear CFHASH instead of risking a corrupt read.
> + */
> +static void ext4_setent_compact_exts(struct ext4_renament *ent,
> + struct ext4_dirent_data_header *lddh,
> + unsigned int lufid_off,
> + unsigned int rec_len)
> +{
> + struct ext4_dirent_data_header *cddh;
> + unsigned int coff;
> +
> + if (!(ent->de->file_type & EXT4_DIRENT_CFHASH))
> + return;
> + if (lufid_off + sizeof(*lddh) > rec_len ||
> + !lddh->ddh_length ||
> + lufid_off + lddh->ddh_length > rec_len) {
> + ent->de->file_type &= ~EXT4_DIRENT_CFHASH;
> + return;
> + }
> +
> + coff = lufid_off + lddh->ddh_length;
> + cddh = (struct ext4_dirent_data_header *)((char *)ent->de + coff);
> +
> + if (coff + sizeof(*cddh) > rec_len ||
> + !cddh->ddh_length ||
> + coff + cddh->ddh_length > rec_len) {
> + ent->de->file_type &= ~EXT4_DIRENT_CFHASH;
> + return;
> + }
> +
> + memmove(lddh, cddh, cddh->ddh_length);
> +}
> +
> static int ext4_setent(handle_t *handle, struct ext4_renament *ent,
> unsigned ino, unsigned file_type,
> const struct ext4_dirent_fid *src_fid)
> @@ -3786,13 +4051,25 @@ static int ext4_setent(handle_t *handle, struct ext4_renament *ent,
> if (ddh_off + sizeof(*ddh) > rec_len ||
> ddh->ddh_length != copy_len ||
> ddh_off + copy_len > rec_len) {
> - /* Cannot copy: clear the flag so the
> - * slot does not advertise a stale LUFID
> - * from the old inode. */
> + /* Cannot copy in-place (size mismatch).
> + * If CFHASH is also present, slide it
> + * into the LUFID slot so the parser
> + * finds it at the correct offset after
> + * the LUFID flag is cleared. */
> + ext4_setent_compact_exts(ent, ddh,
> + ddh_off, rec_len);
> ent->de->file_type &= ~EXT4_DIRENT_LUFID;
> } else {
> memcpy(ddh, src_fid, copy_len);
> }
> + } else if (src_fid) {
> + /* Destination has no LUFID slot; cannot
> + * propagate in-place. Clear the flag from
> + * the on-disk entry directly — clearing only
> + * the local file_type variable has no effect
> + * because the final assignment takes the high
> + * bits from ent->de->file_type, not file_type. */
> + ent->de->file_type &= ~EXT4_DIRENT_LUFID;
> } else if (!src_fid) {
> /* Sync the LUFID flag with what file_type requests.
> * For normal rename (source has no LUFID) and for
> @@ -3800,7 +4077,24 @@ static int ext4_setent(handle_t *handle, struct ext4_renament *ent,
> * so we clear the stale flag. For ext4_resetent
> * (error recovery), file_type is the original
> * file_type with LUFID=1, so we restore the flag —
> - * the LUFID bytes are still on disk untouched. */
> + * the LUFID bytes are still on disk untouched.
> + * When clearing LUFID and CFHASH is present, slide
> + * CFHASH bytes into the LUFID slot first so the
> + * parser finds CFHASH at the expected offset. */
> + if (!(file_type & EXT4_DIRENT_LUFID) &&
> + (ent->de->file_type & EXT4_DIRENT_LUFID)) {
> + unsigned int loff =
> + EXT4_BASE_DIR_LEN +
> + ent->de->name_len + 1;
> + unsigned int rlen =
> + ext4_rec_len_from_disk(
> + ent->de->rec_len,
> + ent->dir->i_sb->s_blocksize);
> + ext4_setent_compact_exts(ent,
> + (struct ext4_dirent_data_header *)
> + ((char *)ent->de + loff),
> + loff, rlen);
> + }
> ent->de->file_type =
> (ent->de->file_type & ~EXT4_DIRENT_LUFID) |
> (file_type & EXT4_DIRENT_LUFID);
[Severity: High]
Does calling ext4_setent_compact_exts() here corrupt old.de when a
RENAME_WHITEOUT rename fails and rolls back via ext4_resetent()?
In fs/ext4/namei.c:ext4_rename(), when whiteout is non-NULL and old.de has
both EXT4_DIRENT_LUFID and EXT4_DIRENT_CFHASH set, ext4_rename() first
updates old.de to point to the whiteout inode before adding the new entry:
if (whiteout) {
retval = ext4_setent(handle, &old, whiteout->i_ino,
EXT4_FT_CHRDEV, NULL);
if (retval)
goto end_rename;
}
if (!new.de) {
retval = ext4_add_entry(handle, new.dentry, old.inode,
old_fid);
Inside that first ext4_setent() call, src_fid is NULL and file_type is
EXT4_FT_CHRDEV (which does not have EXT4_DIRENT_LUFID set), so
ext4_setent_compact_exts() memmoves the 9-byte CFHASH extension over the
on-disk LUFID slot at loff = EXT4_BASE_DIR_LEN + old.de->name_len + 1.
If ext4_add_entry() (or a subsequent step) fails, execution jumps to
end_rename:
end_rename:
kfree(edp);
ext4_lufid_snap_release(&old_snap);
if (whiteout) {
if (retval) {
ext4_resetent(handle, &old,
old.inode->i_ino, old_file_type);
ext4_resetent() calls ext4_setent(handle, &old, old.inode->i_ino,
old_file_type, NULL). The comment in the !src_fid branch assumes the LUFID
bytes are still on disk untouched, and sets EXT4_DIRENT_LUFID back to 1 in
old.de->file_type. However, the LUFID slot was already overwritten with the
CFHASH bytes during the whiteout step, leaving old.de with both
EXT4_DIRENT_LUFID and EXT4_DIRENT_CFHASH set in file_type, the CFHASH sitting
in the LUFID slot, and stale bytes at the post-LUFID offset where
ext4_dirdata_get() will look for CFHASH.
Also, in ext4_setent_compact_exts(), should coff account for
EXT4_DIRENT_INO64 if that flag is set between EXT4_DIRENT_LUFID and
EXT4_DIRENT_CFHASH, matching the layout in ext4_dirdata_get()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029122413.230409-1-ablagodarenko@thelustrecollective.com?part=10
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v13 11/11] ext4: Add EXT4_IOC_SET_LUFID ioctl for setting LUFID on directory entries
2026-10-29 12:23 [PATCH v13 00/11] Data in direntry (dirdata) feature Artem Blagodarenko
` (9 preceding siblings ...)
2026-10-29 12:24 ` [PATCH v13 10/11] ext4: add dirdata set/get helpers Artem Blagodarenko
@ 2026-10-29 12:24 ` Artem Blagodarenko
2026-10-29 12:58 ` sashiko-bot
10 siblings, 1 reply; 23+ messages in thread
From: Artem Blagodarenko @ 2026-10-29 12:24 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, jack, adilger.kernel, ablagodarenko, Artem Blagodarenko,
Andreas Dilger
From: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Add a new ioctl command that allows setting LUFID (Locally Unique File ID)
data on existing directory entries. This includes:
- ext4_ioctl_set_lufid(): ioctl handler that validates parameters and
calls the underlying implementation
- ext4_set_direntry_lufid(): Core function that performs the operation by:
* Looking up the target directory entry
* Retrieving the associated inode
* Deleting the old entry and re-creating it with LUFID data attached
This implementation requires the dirdata feature to be enabled on the
filesystem and properly handles transactions and inode locking to ensure
consistency.
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
---
fs/ext4/dir.c | 3 +-
fs/ext4/ext4.h | 23 +--
fs/ext4/inline.c | 4 +-
fs/ext4/ioctl.c | 85 ++++++++++
fs/ext4/namei.c | 347 +++++++++++++++++++++++++++++++++++---
include/uapi/linux/ext4.h | 14 ++
6 files changed, 442 insertions(+), 34 deletions(-)
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 12df2d2a0997..dca6796d5fe2 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -507,7 +507,8 @@ int ext4_htree_store_dirent(struct file *dir_file, __u32 hash,
p = &info->root.rb_node;
/* Create and allocate the fname structure */
- if (dirent->file_type & ~EXT4_FT_MASK) {
+ if (ext4_has_feature_dirdata(dir_file->f_inode->i_sb) &&
+ (dirent->file_type & ~EXT4_FT_MASK)) {
unsigned int rec_len =
ext4_rec_len_from_disk(dirent->rec_len, buf_size);
extra_data = ext4_dirent_get_data_len(dirent, rec_len);
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 086a8a0d340f..87c55276d67a 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3091,17 +3091,17 @@ extern int ext4_find_dest_de(struct inode *dir, struct buffer_head *bh,
struct ext4_filename *fname,
struct ext4_dir_entry_2 **dest_de,
int dlen);
-void ext4_insert_dentry_data(struct inode *dir, struct inode *inode,
- struct ext4_dir_entry_2 *de,
- int buf_size,
- struct ext4_filename *fname,
- void *data);
-static inline void ext4_insert_dentry(struct inode *dir, struct inode *inode,
- struct ext4_dir_entry_2 *de,
- int buf_size,
- struct ext4_filename *fname)
+int ext4_insert_dentry_data(struct inode *dir, struct inode *inode,
+ struct ext4_dir_entry_2 *de,
+ int buf_size,
+ struct ext4_filename *fname,
+ void *data);
+static inline int ext4_insert_dentry(struct inode *dir, struct inode *inode,
+ struct ext4_dir_entry_2 *de,
+ int buf_size,
+ struct ext4_filename *fname)
{
- ext4_insert_dentry_data(dir, inode, de, buf_size, fname, NULL);
+ return ext4_insert_dentry_data(dir, inode, de, buf_size, fname, NULL);
}
static inline void ext4_update_dx_flag(struct inode *inode)
{
@@ -3369,6 +3369,9 @@ static inline int ext4_init_new_dir(handle_t *handle, struct inode *dir,
}
extern int ext4_dirblock_csum_verify(struct inode *inode,
struct buffer_head *bh);
+extern int ext4_dirdata_set_lufid(struct mnt_idmap *idmap, struct inode *dir,
+ const char *filename, int namelen,
+ struct ext4_dentry_param *edp);
extern int ext4_htree_fill_tree(struct file *dir_file, __u32 start_hash,
__u32 start_minor_hash, __u32 *next_hash);
extern int ext4_search_dir(struct buffer_head *bh,
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 700a0282278b..76b1490490f6 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1004,7 +1004,9 @@ static int ext4_add_dirent_to_inline(handle_t *handle,
EXT4_JTR_NONE);
if (err)
return err;
- ext4_insert_dentry_data(dir, inode, de, inline_size, fname, dfid);
+ err = ext4_insert_dentry_data(dir, inode, de, inline_size, fname, dfid);
+ if (err)
+ return err;
ext4_show_inline_dir(dir, iloc->bh, inline_start, inline_size);
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 1f6d7afb62ec..baf6ef48cbf3 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -1536,6 +1536,88 @@ static int ext4_ioctl_set_tune_sb(struct file *filp,
return ret;
}
+/*
+ * ext4_ioctl_set_lufid() - Set LUFID on a directory entry
+ * @filp: file pointer (parent directory)
+ * @arg: pointer to ext4_set_lufid structure with filename and LUFID data
+ *
+ * This ioctl allows setting LUFID data on an existing
+ * directory entry. It is called on the parent directory with a filename and
+ * LUFID data.
+ */
+static long ext4_ioctl_set_lufid(struct file *filp, unsigned long arg)
+{
+ struct inode *dir = file_inode(filp);
+ struct mnt_idmap *idmap = file_mnt_idmap(filp);
+ struct ext4_set_lufid lufid_args;
+ struct {
+ __u32 edp_magic;
+ struct ext4_dirent_data_header df_header;
+ char df_fid[255];
+ } edp;
+ int err;
+
+ /* Check if parent is a directory */
+ if (!S_ISDIR(dir->i_mode))
+ return -ENOTDIR;
+
+ /* This ioctl mutates directory entries; merely having the directory
+ * open (which only ever requires read access) is not enough.
+ * MAY_EXEC is required for entry lookup; MAY_WRITE for modification. */
+ err = inode_permission(idmap, dir, MAY_WRITE | MAY_EXEC);
+ if (err)
+ return err;
+
+ /* Copy arguments from user space */
+ if (copy_from_user(&lufid_args, (struct ext4_set_lufid __user *)arg,
+ sizeof(lufid_args)))
+ return -EFAULT;
+
+ /* Validate parameters. esl_name_len is NUL-excluded length (1-255). */
+ if (lufid_args.esl_name_len == 0 || lufid_args.esl_name_len > EXT4_NAME_LEN)
+ return -EINVAL;
+
+ /* ddh_length (esl_data_len + the header byte below) must itself fit
+ * in the __u8 ddh_length field without wrapping */
+ if (lufid_args.esl_data_len == 0 ||
+ lufid_args.esl_data_len > 255 - sizeof(edp.df_header))
+ return -EINVAL;
+
+ /* Ensure filename is NUL-terminated at exactly esl_name_len */
+ if (lufid_args.esl_name[lufid_args.esl_name_len] != '\0')
+ return -EINVAL;
+
+ /* '.' and '..' are not ordinary entries -- they must stay the first
+ * two entries in the directory's first block, so they can't go
+ * through the general delete+re-add path this ioctl uses */
+ if (!strcmp(lufid_args.esl_name, ".") || !strcmp(lufid_args.esl_name, ".."))
+ return -EINVAL;
+
+ /* Prepare the dentry param struct with LUFID data. ddh_length is
+ * documented (see struct ext4_dirent_data_header) as the length of
+ * the header plus the whole data blob -- include the header here so
+ * every dirdata reader/writer that takes ddh_length at face value
+ * (e.g. ext4_dirdata_set()'s memcpy) copies the full LUFID payload
+ * instead of silently dropping its last byte. */
+ edp.edp_magic = EXT4_LUFID_MAGIC;
+ edp.df_header.ddh_length = lufid_args.esl_data_len +
+ sizeof(edp.df_header);
+ memcpy(edp.df_fid, lufid_args.esl_data, lufid_args.esl_data_len);
+
+ /* Want write access */
+ err = mnt_want_write_file(filp);
+ if (err)
+ return err;
+
+ /* Call the helper function to do the actual work */
+ err = ext4_dirdata_set_lufid(idmap, dir, lufid_args.esl_name,
+ lufid_args.esl_name_len,
+ (struct ext4_dentry_param *)&edp);
+
+ mnt_drop_write_file(filp);
+ return err;
+}
+
static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
struct inode *inode = file_inode(filp);
@@ -1922,6 +2004,8 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
(void __user *)arg);
case EXT4_IOC_SET_TUNE_SB_PARAM:
return ext4_ioctl_set_tune_sb(filp, (void __user *)arg);
+ case EXT4_IOC_SET_LUFID:
+ return ext4_ioctl_set_lufid(filp, arg);
default:
return -ENOTTY;
}
@@ -2001,6 +2085,7 @@ long ext4_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case FS_IOC_SETFSLABEL:
case EXT4_IOC_GETFSUUID:
case EXT4_IOC_SETFSUUID:
+ case EXT4_IOC_SET_LUFID:
break;
default:
return -ENOIOCTLCMD;
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 5a118320eef9..4ab1f14442b0 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -543,11 +543,13 @@ static struct dx_root_info *dx_get_dx_info(struct inode *dir, void *de_buf)
{
unsigned int blocksize = dir->i_sb->s_blocksize;
void *base = de_buf;
+ unsigned int rlen;
/* '.' and '..' never carry the casefold+fscrypt hash, so pass NULL
* for dir regardless of the directory's flags */
- if (ext4_rec_len_from_disk(((struct ext4_dir_entry_2 *)de_buf)->rec_len,
- blocksize) < EXT4_BASE_DIR_LEN)
+ rlen = ext4_rec_len_from_disk(((struct ext4_dir_entry_2 *)de_buf)->rec_len,
+ blocksize);
+ if (rlen < EXT4_BASE_DIR_LEN || rlen > blocksize)
return ERR_PTR(-EFSCORRUPTED);
de_buf += ext4_dir_entry_len(de_buf, blocksize, NULL);
@@ -555,6 +557,11 @@ static struct dx_root_info *dx_get_dx_info(struct inode *dir, void *de_buf)
if (de_buf < base || (char *)de_buf - (char *)base +
EXT4_BASE_DIR_LEN > blocksize)
return ERR_PTR(-EFSCORRUPTED);
+ rlen = ext4_rec_len_from_disk(((struct ext4_dir_entry_2 *)de_buf)->rec_len,
+ blocksize);
+ if (rlen < EXT4_BASE_DIR_LEN ||
+ (char *)de_buf - (char *)base + rlen > blocksize)
+ return ERR_PTR(-EFSCORRUPTED);
de_buf += ext4_dir_entry_len(de_buf, blocksize, NULL);
if (de_buf < base || (char *)de_buf - (char *)base +
@@ -614,6 +621,9 @@ static inline unsigned dx_root_limit(struct inode *dir,
info = dx_get_dx_info(dir, dot_de);
if (IS_ERR(info))
return 0;
+ if ((char *)info - (char *)dot_de + info->info_length >
+ dir->i_sb->s_blocksize)
+ return 0;
entry_space = dir->i_sb->s_blocksize - ((char *)info - (char *)dot_de) -
info->info_length;
@@ -1485,9 +1495,9 @@ unsigned char ext4_dirdata_get(struct ext4_dir_entry_2 *de, struct inode *dir,
* directories requires an e2fsck migration pass before tune2fs sets the
* EXT4_FEATURE_INCOMPAT_DIRDATA superblock flag.
*/
-static void ext4_dirdata_set(struct ext4_dir_entry_2 *de, struct inode *dir,
- struct ext4_dirent_fid *dfid,
- struct ext4_filename *fname)
+static int ext4_dirdata_set(struct ext4_dir_entry_2 *de, struct inode *dir,
+ struct ext4_dirent_fid *dfid,
+ struct ext4_filename *fname)
{
struct dx_hash_info *hinfo = &fname->hinfo;
unsigned int data_offset = de->name_len + 1;
@@ -1508,7 +1518,7 @@ static void ext4_dirdata_set(struct ext4_dir_entry_2 *de, struct inode *dir,
if (EXT4_BASE_DIR_LEN + data_offset + dlen > rec_len) {
EXT4_ERROR_INODE(dir, "Can not insert FID");
- return;
+ return -EIO;
}
memcpy((char *)de + EXT4_BASE_DIR_LEN + data_offset, dfid, dlen);
@@ -1523,7 +1533,7 @@ static void ext4_dirdata_set(struct ext4_dir_entry_2 *de, struct inode *dir,
if (EXT4_BASE_DIR_LEN + data_offset + sizeof(*dh) > rec_len) {
EXT4_ERROR_INODE(dir, "Can not insert dhash dirdata");
- return;
+ return -EIO;
}
dh->dh_header.ddh_length = sizeof(*dh);
@@ -1531,11 +1541,18 @@ static void ext4_dirdata_set(struct ext4_dir_entry_2 *de, struct inode *dir,
dh->dh_hash.minor_hash = cpu_to_le32(hinfo->minor_hash);
de->file_type |= EXT4_DIRENT_CFHASH;
} else {
- /* Compatibility: store hash inline after filename */
- if (EXT4_BASE_DIR_LEN + data_offset +
- sizeof(struct ext4_dir_entry_hash) > rec_len) {
+ /* Compatibility: store hash inline after filename.
+ * EXT4_DIRENT_HASHES places the hash at the
+ * 4-byte-aligned offset (8 + name_len + 3) & ~3, which
+ * may be less than EXT4_BASE_DIR_LEN + data_offset when
+ * name_len is a multiple of 4 (gap byte counted twice).
+ * Use the same aligned offset for the bounds check. */
+ unsigned int hash_off =
+ (EXT4_BASE_DIR_LEN + de->name_len + 3) & ~3;
+
+ if (hash_off + sizeof(struct ext4_dir_entry_hash) > rec_len) {
EXT4_ERROR_INODE(dir, "Can not insert dhash");
- return;
+ return -EIO;
}
EXT4_DIRENT_HASHES(de)->hash = cpu_to_le32(hinfo->hash);
@@ -1543,6 +1560,7 @@ static void ext4_dirdata_set(struct ext4_dir_entry_2 *de, struct inode *dir,
cpu_to_le32(hinfo->minor_hash);
}
}
+ return 0;
}
/*
@@ -2374,9 +2392,9 @@ int ext4_find_dest_de(struct inode *dir, struct buffer_head *bh,
return 0;
}
-void ext4_insert_dentry_data(struct inode *dir, struct inode *inode,
- struct ext4_dir_entry_2 *de, int buf_size,
- struct ext4_filename *fname, void *data)
+int ext4_insert_dentry_data(struct inode *dir, struct inode *inode,
+ struct ext4_dir_entry_2 *de, int buf_size,
+ struct ext4_filename *fname, void *data)
{
int nlen, rlen;
@@ -2394,7 +2412,7 @@ void ext4_insert_dentry_data(struct inode *dir, struct inode *inode,
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));
- ext4_dirdata_set(de, dir, data, fname);
+ return ext4_dirdata_set(de, dir, data, fname);
}
/*
@@ -2446,7 +2464,11 @@ static int add_dirent_to_buf(handle_t *handle, struct ext4_filename *fname,
}
/* By now the buffer is marked for journaling */
- ext4_insert_dentry_data(dir, inode, de, blocksize, fname, dfid);
+ err = ext4_insert_dentry_data(dir, inode, de, blocksize, fname, dfid);
+ if (err) {
+ ext4_std_error(dir->i_sb, err);
+ return err;
+ }
/*
* XXX shouldn't update any times until successful
@@ -3260,14 +3282,15 @@ int ext4_init_dirblock(handle_t *handle, struct inode *inode,
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;
+ unsigned int dot_rec_len;
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_dirent_rec_len(de->name_len, NULL),
- blocksize);
+ dot_rec_len = ext4_dirent_rec_len(de->name_len, NULL);
+ de->rec_len = ext4_rec_len_to_disk(dot_rec_len, blocksize);
memcpy(de->name, ".", 2);
ext4_set_de_type(inode->i_sb, de, S_IFDIR);
@@ -3287,7 +3310,7 @@ int ext4_init_dirblock(handle_t *handle, struct inode *inode,
blocksize - csum_size);
} else {
de->rec_len = ext4_rec_len_to_disk(blocksize -
- (csum_size + ext4_dirent_rec_len(1, NULL)),
+ (csum_size + dot_rec_len),
blocksize);
}
@@ -3320,6 +3343,12 @@ int ext4_init_new_dir_data(handle_t *handle, struct inode *dir,
dir_block = ext4_append(handle, inode, &block);
if (IS_ERR(dir_block))
return PTR_ERR(dir_block);
+ /*
+ * data1 and data2 are reserved for callers that need to embed
+ * dirdata into the '.' and '..' entries of a new directory.
+ * That path is not yet implemented here; a caller requiring it
+ * must write the dirdata entries after this function returns.
+ */
err = ext4_init_dirblock(handle, inode, dir_block, dir->i_ino, NULL, 0);
out:
brelse(dir_block);
@@ -4454,14 +4483,13 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir,
if (!new.bh && old_snap.fid) {
unsigned fid_size = old_snap.fid->df_header.ddh_length;
- edp = kmalloc(offsetof(struct ext4_dentry_param,
- edp_dfid) + fid_size, GFP_NOFS);
+ edp = kmalloc(sizeof(*edp) + fid_size, GFP_NOFS);
if (!edp) {
retval = -ENOMEM;
goto end_rename;
}
edp->edp_magic = EXT4_LUFID_MAGIC;
- memcpy(&edp->edp_dfid, old_snap.fid, fid_size);
+ memcpy(edp->edp_dfid, old_snap.fid, fid_size);
}
if (whiteout) {
@@ -4581,6 +4609,21 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir,
ext4_lufid_snap_release(&old_snap);
if (whiteout) {
if (retval) {
+ /* Known limitation: if the old entry carried both LUFID
+ * and CFHASH, ext4_setent_compact_exts() slid CFHASH
+ * into the LUFID slot above, and old_snap was released
+ * before this point, so resetent can only restore the
+ * LUFID flag — not the original byte layout. This
+ * leaves the entry with CFHASH data at the LUFID offset
+ * and garbage at the CFHASH offset until the next
+ * SET_LUFID ioctl re-writes it. The scenario requires
+ * RENAME_WHITEOUT on a casefolded+encrypted dirdata
+ * directory whose entry has a LUFID, followed by a
+ * journal IO error in the single ext4_mark_inode_dirty()
+ * call that follows — an extremely unlikely combination
+ * in practice. (OOM during the LUFID wrapper allocation
+ * cannot trigger this path because that allocation is
+ * hoisted before ext4_setent() modifies the entry.) */
ext4_resetent(handle, &old,
old.inode->i_ino, old_file_type);
drop_nlink(whiteout);
@@ -4793,6 +4836,266 @@ static int ext4_rename2(struct mnt_idmap *idmap,
return ext4_rename(idmap, old_dir, old_dentry, new_dir, new_dentry, flags);
}
+/*
+ * ext4_dirdata_set_lufid() - Set LUFID data on an existing directory entry
+ * @dir: parent directory inode
+ * @filename: name of the file in the directory
+ * @namelen: length of filename
+ * @edp: pointer to initialized dentry param with LUFID data
+ *
+ * This function finds an existing directory entry, deletes it, and re-creates it
+ * with LUFID data attached. Used by the EXT4_IOC_SET_LUFID ioctl.
+ *
+ * Returns 0 on success, negative error code on failure.
+ */
+int ext4_dirdata_set_lufid(struct mnt_idmap *idmap, struct inode *dir,
+ const char *filename, int namelen,
+ struct ext4_dentry_param *edp)
+{
+ struct super_block *sb = dir->i_sb;
+ /* zero-init: safe to free on any path */
+ struct ext4_filename fname = {};
+ struct ext4_dir_entry_2 *de = NULL;
+ struct buffer_head *bh = NULL;
+ struct inode *inode = NULL;
+ handle_t *handle = NULL;
+ struct ext4_lufid_snap old_snap = {};
+ struct qstr d_name;
+ /* on-disk name snapshot for non-encrypted casefolded directories: the
+ * on-disk case may differ from what the caller supplied. */
+ char ondisk_name[EXT4_NAME_LEN];
+ struct qstr real_name = {};
+ /* name to pass to ext4_add_entry() for the re-add and rollback */
+ const struct qstr *add_name;
+ __u32 ino = 0;
+ bool child_locked = false;
+ int err = 0;
+
+ if (!ext4_has_feature_dirdata(sb))
+ return -EOPNOTSUPP;
+
+ if (namelen > EXT4_NAME_LEN)
+ return -ENAMETOOLONG;
+ if (namelen != strnlen(filename, namelen + 1))
+ return -EINVAL;
+
+ d_name.name = filename;
+ d_name.len = namelen;
+
+ err = ext4_fname_setup_filename(dir, &d_name, 0, &fname);
+ if (err)
+ goto out_free;
+
+ /* Lock dir with the VFS parent-mutation subclass. The lookup and any
+ * mutation must be inside this lock to prevent TOCTOU races. */
+ inode_lock_nested(dir, I_MUTEX_PARENT);
+
+ /* Phase 1: look up the entry without holding a journal handle to
+ * obtain the child inode reference before taking the child lock.
+ * VFS locking order requires all inode locks to be acquired before
+ * starting a jbd2 transaction; starting the journal while already
+ * holding the child lock would invert that order and risk deadlock
+ * if jbd2 blocks waiting for a commit that the child lock holder
+ * prevents from finishing. */
+ bh = ext4_find_entry(dir, &d_name, &de, NULL);
+ if (IS_ERR(bh)) {
+ err = PTR_ERR(bh);
+ bh = NULL;
+ goto out_unlock_dir;
+ }
+ if (!bh) {
+ err = -ENOENT;
+ goto out_unlock_dir;
+ }
+ ino = le32_to_cpu(de->inode);
+ brelse(bh);
+ bh = NULL;
+ de = NULL;
+
+ inode = ext4_iget(sb, ino, EXT4_IGET_NORMAL);
+ if (IS_ERR(inode)) {
+ err = PTR_ERR(inode);
+ inode = NULL;
+ goto out_unlock_dir;
+ }
+
+ /* Enforce sticky-bit restriction: on a sticky directory only the
+ * directory owner, the file owner, or a privileged process may
+ * modify entries. inode_permission(MAY_WRITE) alone does not cover
+ * this, so check it explicitly here as may_delete() does. */
+ err = check_sticky(idmap, dir, inode);
+ if (err)
+ goto out_iput;
+
+ /* The ioctl calls ext4_delete_entry() on the parent directory, which
+ * bypasses the VFS may_delete() path. inode_permission(MAY_WRITE)
+ * above blocks IS_IMMUTABLE(dir) but does NOT block IS_APPEND(dir):
+ * append-only directories allow new entries but forbid deletion, and
+ * that restriction lives only in may_delete(), not in
+ * inode_permission(). Enforce it explicitly here. */
+ if (IS_APPEND(dir)) {
+ err = -EPERM;
+ goto out_iput;
+ }
+
+ /* The ioctl deletes and re-creates the directory entry, which
+ * effectively modifies the target file's namespace presence.
+ * Refuse if the target itself is immutable or append-only. */
+ if (IS_IMMUTABLE(inode) || IS_APPEND(inode)) {
+ err = -EPERM;
+ goto out_iput;
+ }
+
+ /* Lock the target inode BEFORE starting the journal (dir-then-child,
+ * VFS-locks-before-journal ordering). Use I_MUTEX_CHILD for directory
+ * inodes (consistent with VFS rename/rmdir) and I_MUTEX_NONDIR2 for
+ * all others; mixing subclasses on the same inode triggers lockdep
+ * cycles with concurrent rename. */
+ if (inode != dir) {
+ if (S_ISDIR(inode->i_mode))
+ inode_lock_nested(inode, I_MUTEX_CHILD);
+ else
+ inode_lock_nested(inode, I_MUTEX_NONDIR2);
+ child_locked = true;
+ }
+
+ handle = ext4_journal_start(dir, EXT4_HT_DIR,
+ 3 * EXT4_DATA_TRANS_BLOCKS(sb) +
+ 2 * EXT4_INDEX_EXTRA_TRANS_BLOCKS);
+ if (IS_ERR(handle)) {
+ err = PTR_ERR(handle);
+ handle = NULL;
+ goto out_unlock;
+ }
+
+ /* Phase 2: re-look up the entry inside the journal to obtain a stable
+ * de pointer. Verify it still refers to the same inode (TOCTOU
+ * guard: a concurrent unlink+create between phase 1 and here would
+ * change the inode number). */
+ bh = ext4_find_entry(dir, &d_name, &de, NULL);
+ if (IS_ERR(bh)) {
+ err = PTR_ERR(bh);
+ bh = NULL;
+ goto out_journal;
+ }
+ if (!bh) {
+ err = -ENOENT;
+ goto out_journal;
+ }
+ if (le32_to_cpu(de->inode) != ino) {
+ err = -ENOENT;
+ goto out_brelse;
+ }
+
+ /* Choose the name to use for re-adding the entry after deletion.
+ *
+ * For non-encrypted casefolded directories ext4_find_entry() matches
+ * case-insensitively, so de->name may differ in case from the
+ * caller-supplied filename. Re-adding with the caller's case would
+ * silently rename the entry, so preserve the on-disk bytes.
+ *
+ * For encrypted directories (with or without casefold) de->name holds
+ * the raw ciphertext. ext4_fname_setup_filename() deterministically
+ * re-encrypts d_name (the plaintext) to the identical ciphertext, so
+ * using d_name for the re-add is correct. Passing the raw ciphertext
+ * bytes would cause double-encryption and make the entry unreachable. */
+ if (IS_CASEFOLDED(dir) && !IS_ENCRYPTED(dir)) {
+ memcpy(ondisk_name, de->name, de->name_len);
+ real_name.name = ondisk_name;
+ real_name.len = de->name_len;
+ add_name = &real_name;
+ } else {
+ add_name = &d_name;
+ }
+
+ /* Snapshot the old LUFID before deleting so we can restore it if the
+ * re-add fails. Without this the rollback path re-adds the entry
+ * without any LUFID, silently orphaning the old FID-to-path mapping. */
+ err = ext4_lufid_snapshot(de, sb->s_blocksize, &old_snap);
+ if (err)
+ goto out_brelse;
+
+ err = ext4_delete_entry(handle, dir, de, bh);
+ if (err)
+ goto out_brelse;
+
+ brelse(bh);
+ bh = NULL;
+
+ /* Re-add with LUFID via dentry->d_fsdata. ext4_add_entry() resolves
+ * dfid from d_fsdata into fname.dfid and passes it through to
+ * add_dirent_to_buf() without touching the shared i_dirdata field,
+ * eliminating the race with concurrent link() calls. */
+ {
+ struct dentry parent_dentry = { .d_inode = dir };
+ struct dentry new_dentry = {
+ .d_name = *add_name,
+ .d_parent = &parent_dentry,
+ .d_inode = inode,
+ .d_fsdata = edp,
+ };
+ err = ext4_add_entry(handle, &new_dentry, inode);
+ }
+
+ if (err) {
+ /* Delete succeeded but re-add failed; try to restore so the
+ * inode is not left without a directory entry. Restore the
+ * original LUFID too: without it the entry is re-added clean
+ * and the old FID-to-path mapping is silently lost. */
+ struct dentry parent_dentry = { .d_inode = dir };
+ struct dentry orig_dentry = {
+ .d_name = *add_name,
+ .d_parent = &parent_dentry,
+ .d_inode = inode,
+ };
+ int rollback_err;
+
+ if (old_snap.fid) {
+ unsigned int fid_size = old_snap.fid->df_header.ddh_length;
+ struct ext4_dentry_param *old_edp =
+ kmalloc(sizeof(*old_edp) + fid_size, GFP_NOFS);
+
+ if (old_edp) {
+ old_edp->edp_magic = EXT4_LUFID_MAGIC;
+ memcpy(old_edp->edp_dfid, old_snap.fid, fid_size);
+ orig_dentry.d_fsdata = old_edp;
+ }
+ }
+
+ rollback_err = ext4_add_entry(handle, &orig_dentry, inode);
+ kfree(orig_dentry.d_fsdata);
+
+ if (rollback_err)
+ EXT4_ERROR_INODE(dir,
+ "Failed to set LUFID on '%.*s' (err=%d) and failed to restore the original directory entry (err=%d); inode %llu may be orphaned",
+ namelen, filename, err, rollback_err,
+ (unsigned long long)inode->i_ino);
+ goto out_journal;
+ }
+
+ inode_set_ctime_current(dir);
+ inode_inc_iversion(dir);
+ ext4_mark_inode_dirty(handle, dir);
+
+out_brelse:
+ brelse(bh);
+out_journal:
+ ext4_lufid_snap_release(&old_snap);
+ if (handle)
+ ext4_journal_stop(handle);
+out_unlock:
+ if (child_locked)
+ inode_unlock(inode);
+out_iput:
+ iput(inode);
+out_unlock_dir:
+ inode_unlock(dir);
+out_free:
+ ext4_fname_free_filename(&fname);
+
+ return err;
+}
+
/*
* directories can handle most operations...
*/
diff --git a/include/uapi/linux/ext4.h b/include/uapi/linux/ext4.h
index 9c683991c32f..9134fe64947a 100644
--- a/include/uapi/linux/ext4.h
+++ b/include/uapi/linux/ext4.h
@@ -35,6 +35,7 @@
#define EXT4_IOC_SETFSUUID _IOW('f', 44, struct fsuuid)
#define EXT4_IOC_GET_TUNE_SB_PARAM _IOR('f', 45, struct ext4_tune_sb_params)
#define EXT4_IOC_SET_TUNE_SB_PARAM _IOW('f', 46, struct ext4_tune_sb_params)
+#define EXT4_IOC_SET_LUFID _IOW('f', 47, struct ext4_set_lufid)
#define EXT4_IOC_SHUTDOWN _IOR('X', 125, __u32)
@@ -92,6 +93,19 @@ struct move_extent {
__u64 moved_len; /* moved block length */
};
+/*
+ * Structure for EXT4_IOC_SET_LUFID
+ * Sets LUFID on a directory entry
+ * Called on parent directory with filename and LUFID data as arguments
+ */
+struct ext4_set_lufid {
+ __u8 esl_name_len; /* length of filename, NOT including NUL terminator
+ * (valid range: 1-255, matching EXT4_NAME_LEN) */
+ char esl_name[255 + 1]; /* filename (NUL-terminated) */
+ __u8 esl_data_len; /* length of LUFID data */
+ char esl_data[255]; /* LUFID data (raw bytes) */
+};
+
/*
* Flags used by EXT4_IOC_SHUTDOWN
*/
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v13 11/11] ext4: Add EXT4_IOC_SET_LUFID ioctl for setting LUFID on directory entries
2026-10-29 12:24 ` [PATCH v13 11/11] ext4: Add EXT4_IOC_SET_LUFID ioctl for setting LUFID on directory entries Artem Blagodarenko
@ 2026-10-29 12:58 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-10-29 12:58 UTC (permalink / raw)
To: Artem Blagodarenko, linux-ext4; +Cc: tytso
Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [Critical] ext4: buffer overflow in ext4_insert_dentry_data() with large LUFID entries
- [High] ext4: missing dquot_initialize() in ext4_dirdata_set_lufid()
- [High] ext4: silent LUFID loss and inode orphan risk in ext4_dirdata_set_lufid()
- [High] ext4: ext4_dirdata_set_lufid() deletes dirent before strict UTF-8 check
- [High] ext4: directory entry corruption in ext4_rename() whiteout rollback
- [Medium] ext4: ext4_dirdata_set_lufid() changes filename case on encrypted casefold dirs
- [Medium] ext4: inconsistent dirent length and offset calculation in dx_get_dx_info()
- [Medium] ext4: missing IS_DIRSYNC() check in ext4_dirdata_set_lufid()
- [Medium] ext4: potential i_rwsem deadlock on subdir in ext4_dirdata_set_lufid()
--
commit cdb77af430b2349668752f74c28aae8c30a2dce5
Author: Artem Blagodarenko <artem.blagodarenko@gmail.com>
ext4: Add EXT4_IOC_SET_LUFID ioctl for setting LUFID on directory entries
This commit adds the EXT4_IOC_SET_LUFID ioctl and ext4_dirdata_set_lufid() to
set LUFID dirdata on an existing directory entry by looking up the target
entry, deleting it, and re-creating it with the LUFID payload attached.
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 5a118320eef9b..4ab1f14442b09 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -543,11 +543,13 @@ static struct dx_root_info *dx_get_dx_info(struct inode *dir, void *de_buf)
> {
> unsigned int blocksize = dir->i_sb->s_blocksize;
> void *base = de_buf;
> + unsigned int rlen;
>
> /* '.' and '..' never carry the casefold+fscrypt hash, so pass NULL
> * for dir regardless of the directory's flags */
> - if (ext4_rec_len_from_disk(((struct ext4_dir_entry_2 *)de_buf)->rec_len,
> - blocksize) < EXT4_BASE_DIR_LEN)
> + rlen = ext4_rec_len_from_disk(((struct ext4_dir_entry_2 *)de_buf)->rec_len,
> + blocksize);
> + if (rlen < EXT4_BASE_DIR_LEN || rlen > blocksize)
> return ERR_PTR(-EFSCORRUPTED);
> de_buf += ext4_dir_entry_len(de_buf, blocksize, NULL);
>
> @@ -555,6 +557,11 @@ static struct dx_root_info *dx_get_dx_info(struct inode *dir, void *de_buf)
> if (de_buf < base || (char *)de_buf - (char *)base +
> EXT4_BASE_DIR_LEN > blocksize)
> return ERR_PTR(-EFSCORRUPTED);
> + rlen = ext4_rec_len_from_disk(((struct ext4_dir_entry_2 *)de_buf)->rec_len,
> + blocksize);
> + if (rlen < EXT4_BASE_DIR_LEN ||
> + (char *)de_buf - (char *)base + rlen > blocksize)
> + return ERR_PTR(-EFSCORRUPTED);
> de_buf += ext4_dir_entry_len(de_buf, blocksize, NULL);
[Severity: Medium]
In dx_get_dx_info(), rlen (from de->rec_len) is validated for both "." and
"..", but de_buf is still advanced by ext4_dir_entry_len(de_buf, blocksize,
NULL) without checking that ext4_dir_entry_len() <= rlen.
Meanwhile, get_dx_countlimit() and make_indexed_dir() locate ".." by stepping
over "." using dot_de->rec_len (rlen), and both ext4_check_dir_entry() and
ext4_check_dx_root() permit dot_de->rec_len > ext4_dir_entry_len(dot_de).
If "." has slack space (or a corrupted name_len where ext4_dir_entry_len() >
rlen), in make_indexed_dir() after bh2 is allocated, EXT4_INODE_INDEX is set
on dir, and entries after ".." are cleared from block 0, dx_get_dx_info() will
step into the slack space of "." instead of dotdot_de at offset rlen. Could
this either return -EFSCORRUPTED (leaving EXT4_INODE_INDEX set on a wiped root
block) or return a dx_root_info pointer that overlaps dotdot_de and corrupts
the root block?
Also, while this commit guards ext4_dirent_get_data_len() with
ext4_has_feature_dirdata() in ext4_htree_store_dirent(), ext4_dir_entry_len()
in fs/ext4/ext4.h still calls ext4_dirent_get_data_len() unconditionally (and
receives dir == NULL here and in get_dx_countlimit()). Should
ext4_dir_entry_len() check ext4_has_feature_dirdata() as well, and should
dx_get_dx_info() verify ext4_dir_entry_len() <= rlen (and advance past "." by
rlen)?
[ ... ]
> @@ -2374,9 +2392,9 @@ int ext4_find_dest_de(struct inode *dir, struct buffer_head *bh,
> return 0;
> }
>
> -void ext4_insert_dentry_data(struct inode *dir, struct inode *inode,
> - struct ext4_dir_entry_2 *de, int buf_size,
> - struct ext4_filename *fname, void *data)
> +int ext4_insert_dentry_data(struct inode *dir, struct inode *inode,
> + struct ext4_dir_entry_2 *de, int buf_size,
> + struct ext4_filename *fname, void *data)
> {
> int nlen, rlen;
>
> @@ -2394,7 +2412,7 @@ void ext4_insert_dentry_data(struct inode *dir, struct inode *inode,
> 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));
> - ext4_dirdata_set(de, dir, data, fname);
> + return ext4_dirdata_set(de, dir, data, fname);
> }
>
> /*
> @@ -2446,7 +2464,11 @@ static int add_dirent_to_buf(handle_t *handle, struct ext4_filename *fname,
> }
>
> /* By now the buffer is marked for journaling */
> - ext4_insert_dentry_data(dir, inode, de, blocksize, fname, dfid);
> + err = ext4_insert_dentry_data(dir, inode, de, blocksize, fname, dfid);
> + if (err) {
> + ext4_std_error(dir->i_sb, err);
> + return err;
> + }
[Severity: Critical]
Can this overflow bh->b_data or corrupt the directory block in
ext4_insert_dentry_data() when splitting a 1024-byte directory block containing
large LUFID entries?
In fs/ext4/ioctl.c:ext4_ioctl_set_lufid(), esl_name_len can be up to 255 bytes
and esl_data_len can be up to 254 bytes (ddh_length = 255):
if (lufid_args.esl_name_len == 0 || lufid_args.esl_name_len > EXT4_NAME_LEN)
return -EINVAL;
if (lufid_args.esl_data_len == 0 ||
lufid_args.esl_data_len > 255 - sizeof(edp.df_header))
return -EINVAL;
Including EXT4_BASE_DIR_LEN (8), the 1-byte NUL separator, and an optional
9-byte CFHASH (when casefold and encryption are enabled), a single directory
entry can now require up to 520 bytes (or 528 bytes with CFHASH).
When adding an entry overflows a leaf block, make_indexed_dir() and
ext4_dx_add_entry() call do_split() and pass its returned tail entry de
(non-NULL) to add_dirent_to_buf(handle, fname, dir, inode, de, bh). Because de
is non-NULL, add_dirent_to_buf() skips ext4_find_dest_de() and calls
ext4_insert_dentry_data() directly without verifying that rlen - nlen in de can
hold the new entry:
/* Ensure that neither split block is over half full */
size = 0;
move = 0;
for (i = count-1; i >= 0; i--) {
/* is more than half of this entry in 2nd half of the block? */
if (size + map[i].size/2 > blocksize/2)
break;
size += map[i].size;
move++;
}
On a 1024-byte blocksize filesystem (blocksize / 2 = 512, usable space = 1012
bytes with metadata_csum), do_split() relies on the assumption that a directory
entry is at most 264 bytes. With entries up to 520-528 bytes:
1. If a leaf block holds a single entry >= 496 bytes (count == 1) and a second
~520-byte entry is added, map[0].size / 2 <= 512 gives move = 1 and split =
count - move = 0 (with fallback split = count / 2 = 0), triggering
WARN_ON_ONCE(split == 0), ext4_error_inode_block(), and -EFSCORRUPTED.
2. If a leaf block contains 3 entries of sizes 240, 520 (with LUFID), and 252
bytes (total 1012), do_split() moves both the 252-byte and 520-byte entries
into data2 (since 252 + 520/2 = 512 <= 512), occupying 772 bytes in data2 and
leaving only rlen - nlen = 240 bytes free at offset 772 (only 252 bytes before
the end of the 1024-byte bh2->b_data buffer). When the new entry hashes to
data2 with fname_len(fname) == 255, ext4_insert_dentry_data() places de1 at
offset 772 and calls memcpy(de->name, fname_name(fname), 255) at offset 780
without checking rlen - nlen, overwriting ext4_dir_entry_tail and writing 11
bytes past the end of bh2->b_data.
3. Even when rlen - nlen >= 8 + fname_len(fname) so memcpy(de->name, ...) stays
within bh->b_data, if rlen - nlen is smaller than the full entry size with
LUFID/CFHASH (for example, 496 bytes free vs 520 bytes needed),
ext4_insert_dentry_data() has already mutated the journaled buffer in-place
(shrinking de->rec_len and writing de1->inode, de1->file_type, de1->name_len,
and de1->name) before ext4_dirdata_set() returns -EIO and add_dirent_to_buf()
calls ext4_std_error(dir->i_sb, err). That leaves a half-written active dirent
in the block and causes the rollback ext4_add_entry() in
ext4_dirdata_set_lufid() to fail with -EEXIST.
[ ... ]
> @@ -4581,6 +4609,21 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir,
> ext4_lufid_snap_release(&old_snap);
> if (whiteout) {
> if (retval) {
> + /* Known limitation: if the old entry carried both LUFID
> + * and CFHASH, ext4_setent_compact_exts() slid CFHASH
> + * into the LUFID slot above, and old_snap was released
> + * before this point, so resetent can only restore the
> + * LUFID flag — not the original byte layout. This
> + * leaves the entry with CFHASH data at the LUFID offset
> + * and garbage at the CFHASH offset until the next
> + * SET_LUFID ioctl re-writes it. The scenario requires
> + * RENAME_WHITEOUT on a casefolded+encrypted dirdata
> + * directory whose entry has a LUFID, followed by a
> + * journal IO error in the single ext4_mark_inode_dirty()
> + * call that follows — an extremely unlikely combination
> + * in practice. (OOM during the LUFID wrapper allocation
> + * cannot trigger this path because that allocation is
> + * hoisted before ext4_setent() modifies the entry.) */
> ext4_resetent(handle, &old,
> old.inode->i_ino, old_file_type);
> drop_nlink(whiteout);
[Severity: High]
Does this whiteout rollback path in ext4_rename() permanently corrupt old.de
even without a journal IO error, and can SET_LUFID actually recover it?
After the whiteout block in ext4_rename() calls ext4_setent() (which calls
ext4_setent_compact_exts() and slides the 9-byte CFHASH into the LUFID slot at
loff) and ext4_mark_inode_dirty(handle, whiteout), execution continues to:
if (!new.bh) {
new.dentry->d_fsdata = edp;
retval = ext4_add_entry(handle, new.dentry, old.inode);
new.dentry->d_fsdata = NULL;
if (retval)
goto end_rename;
If ext4_add_entry() fails under normal conditions such as -ENOSPC or -EDQUOT
when allocating a block for the destination directory, it jumps to end_rename.
At end_rename, ext4_lufid_snap_release(&old_snap) frees old_snap before the
if (whiteout) block runs, and ext4_resetent() calls ext4_setent() with dfid ==
NULL, which restores the EXT4_DIRENT_LUFID flag in old.de->file_type while the
9-byte CFHASH (with dh_header.ddh_length == 9) is still sitting at the LUFID
offset loff.
Once that happens, any subsequent lookup (including ext4_find_entry() in
ext4_dirdata_set_lufid()) calls ext4_match() -> ext4_dirdata_get(), which skips
9 bytes for LUFID and reads stale bytes at loff + 9 as the CFHASH. Doesn't the
hash check in ext4_match() then fail on every lookup, making the entry
permanently unreachable to lookup, unlink, or EXT4_IOC_SET_LUFID while leaving
the directory non-empty? Why not pass old_snap.fid to ext4_resetent() and
release old_snap afterwards?
> @@ -4793,6 +4836,266 @@ static int ext4_rename2(struct mnt_idmap *idmap,
> return ext4_rename(idmap, old_dir, old_dentry, new_dir, new_dentry, flags);
> }
>
> +/*
> + * ext4_dirdata_set_lufid() - Set LUFID data on an existing directory entry
> + * @dir: parent directory inode
> + * @filename: name of the file in the directory
> + * @namelen: length of filename
> + * @edp: pointer to initialized dentry param with LUFID data
> + *
> + * This function finds an existing directory entry, deletes it, and re-creates it
> + * with LUFID data attached. Used by the EXT4_IOC_SET_LUFID ioctl.
> + *
> + * Returns 0 on success, negative error code on failure.
> + */
> +int ext4_dirdata_set_lufid(struct mnt_idmap *idmap, struct inode *dir,
> + const char *filename, int namelen,
> + struct ext4_dentry_param *edp)
> +{
[ ... ]
> + d_name.name = filename;
> + d_name.len = namelen;
> +
> + err = ext4_fname_setup_filename(dir, &d_name, 0, &fname);
> + if (err)
> + goto out_free;
> +
> + /* Lock dir with the VFS parent-mutation subclass. The lookup and any
> + * mutation must be inside this lock to prevent TOCTOU races. */
> + inode_lock_nested(dir, I_MUTEX_PARENT);
[Severity: High]
Should ext4_dirdata_set_lufid() call dquot_initialize(dir) before starting the
journal transaction?
Opening a directory via ext4_dir_open() and looking up an inode via ext4_iget()
do not initialize quotas on dir, leaving dir->i_dquot[] entries NULL if no
prior VFS mutation has initialized them. Every other directory-mutating
operation in fs/ext4/namei.c calls dquot_initialize(dir) before
ext4_journal_start().
When ext4_dirdata_set_lufid() deletes the entry and re-adds it with LUFID data
via ext4_add_entry(), the larger entry may not fit in existing inline data or
directory blocks, causing ext4_add_entry() to allocate new directory blocks
(via ext4_convert_inline_data_nolock(), make_indexed_dir(), do_split(), or
ext4_append()). Without dquot_initialize(dir), dquot_alloc_block(dir, ...)
skips quota enforcement and does not charge the allocated blocks while
dir->i_blocks is incremented, which allows bypassing quota limits and causes
quota counter underflow (and warnings in dquot_free_space_nodirty() or
ext4_expand_extra_isize_ea()) when quotas on dir are later initialized and the
blocks are freed.
[ ... ]
> + /* Lock the target inode BEFORE starting the journal (dir-then-child,
> + * VFS-locks-before-journal ordering). Use I_MUTEX_CHILD for directory
> + * inodes (consistent with VFS rename/rmdir) and I_MUTEX_NONDIR2 for
> + * all others; mixing subclasses on the same inode triggers lockdep
> + * cycles with concurrent rename. */
> + if (inode != dir) {
> + if (S_ISDIR(inode->i_mode))
> + inode_lock_nested(inode, I_MUTEX_CHILD);
> + else
> + inode_lock_nested(inode, I_MUTEX_NONDIR2);
> + child_locked = true;
> + }
[Severity: Medium]
Can locking a directory inode with inode_lock_nested(inode, I_MUTEX_CHILD) here
cause an AB-BA deadlock on i_rwsem if a corrupted on-disk directory entry in
dir references an ancestor directory?
In normal VFS path walks, ext4_lookup() -> d_splice_alias() checks
d_ancestor() under rename_lock and rejects directory loops with -ELOOP before
callers like vfs_rmdir() or vfs_rename() lock the child directory inode. Here,
ext4_dirdata_set_lufid() reads ino directly from the on-disk de->inode via
ext4_find_entry() and ext4_iget() while holding dir->i_rwsem, bypassing dcache
ancestor checks and sb->s_vfs_rename_mutex. If de->inode points to an ancestor
directory, locking inode->i_rwsem while holding dir->i_rwsem inverts the
ancestor-before-descendant locking order against concurrent path walks, rmdir,
or rename.
Since deleting and re-adding the entry within the same parent directory only
mutates dir's blocks and does not modify a child directory inode, its i_nlink,
or its ".." entry (similar to same-directory rename in vfs_rename() where
lock_old_subdir is false when new_dir == old_dir), is locking a subdirectory
inode needed here at all?
> +
> + handle = ext4_journal_start(dir, EXT4_HT_DIR,
> + 3 * EXT4_DATA_TRANS_BLOCKS(sb) +
> + 2 * EXT4_INDEX_EXTRA_TRANS_BLOCKS);
> + if (IS_ERR(handle)) {
> + err = PTR_ERR(handle);
> + handle = NULL;
> + goto out_unlock;
> + }
[Severity: Medium]
Should ext4_dirdata_set_lufid() check IS_DIRSYNC(dir) and call
ext4_handle_sync(handle) after starting the transaction, as the other
directory-mutating operations in fs/ext4/namei.c do? Without it, neither
ext4_delete_entry() nor ext4_add_entry() marks the handle synchronous, so
ext4_journal_stop() will return without committing the transaction on a
synchronous directory.
[ ... ]
> + /* Choose the name to use for re-adding the entry after deletion.
> + *
> + * For non-encrypted casefolded directories ext4_find_entry() matches
> + * case-insensitively, so de->name may differ in case from the
> + * caller-supplied filename. Re-adding with the caller's case would
> + * silently rename the entry, so preserve the on-disk bytes.
> + *
> + * For encrypted directories (with or without casefold) de->name holds
> + * the raw ciphertext. ext4_fname_setup_filename() deterministically
> + * re-encrypts d_name (the plaintext) to the identical ciphertext, so
> + * using d_name for the re-add is correct. Passing the raw ciphertext
> + * bytes would cause double-encryption and make the entry unreachable. */
> + if (IS_CASEFOLDED(dir) && !IS_ENCRYPTED(dir)) {
> + memcpy(ondisk_name, de->name, de->name_len);
> + real_name.name = ondisk_name;
> + real_name.len = de->name_len;
> + add_name = &real_name;
> + } else {
> + add_name = &d_name;
> + }
[Severity: Medium]
When both IS_CASEFOLDED(dir) and IS_ENCRYPTED(dir) are true, doesn't using
add_name = &d_name overwrite the on-disk filename's case with the caller's
case?
In ext4_fname_setup_filename(), fscrypt_setup_filename() encrypts the exact
un-casefolded plaintext name (for example, "FooBar" -> Enc("FooBar")), while
ext4_fname_setup_ci_filename() computes the hash of the casefolded name
("foobar") for CFHASH. When ext4_find_entry() is called with d_name = "foobar",
ext4_match() matches the CFHASH and generic_ci_match() decrypts de->name to
"FooBar" and matches it case-insensitively.
Because fscrypt encryption is deterministic on the exact plaintext bytes rather
than the casefolded bytes, re-adding the entry with add_name = &d_name will
encrypt "foobar" into Enc("foobar") != Enc("FooBar"), silently changing the
on-disk filename's casing (and potentially its byte length for multi-byte UTF-8
casefold mappings) and leaving any cached positive dentry in the VFS dcache out
of sync with the on-disk entry.
> +
> + /* Snapshot the old LUFID before deleting so we can restore it if the
> + * re-add fails. Without this the rollback path re-adds the entry
> + * without any LUFID, silently orphaning the old FID-to-path mapping. */
> + err = ext4_lufid_snapshot(de, sb->s_blocksize, &old_snap);
> + if (err)
> + goto out_brelse;
> +
> + err = ext4_delete_entry(handle, dir, de, bh);
> + if (err)
> + goto out_brelse;
> +
> + brelse(bh);
> + bh = NULL;
> +
> + /* Re-add with LUFID via dentry->d_fsdata. ext4_add_entry() resolves
> + * dfid from d_fsdata into fname.dfid and passes it through to
> + * add_dirent_to_buf() without touching the shared i_dirdata field,
> + * eliminating the race with concurrent link() calls. */
> + {
> + struct dentry parent_dentry = { .d_inode = dir };
> + struct dentry new_dentry = {
> + .d_name = *add_name,
> + .d_parent = &parent_dentry,
> + .d_inode = inode,
> + .d_fsdata = edp,
> + };
> + err = ext4_add_entry(handle, &new_dentry, inode);
> + }
[Severity: High]
What happens on a casefolded directory with strict encoding enabled
(SB_ENC_STRICT_MODE_FL) if an existing entry has invalid UTF-8 bytes in its
name (for instance, created before strict encoding was enabled)?
In ext4_dirdata_set_lufid(), ext4_fname_setup_filename() ->
ext4_fname_setup_ci_filename() ignores utf8_casefold() failure by setting
cf_name->name = NULL and returning 0, and ext4_find_entry() ->
generic_ci_match() matches the existing entry via its exact byte-for-byte
memcmp() fast path without checking sb_has_strict_encoding().
However, after ext4_delete_entry() deletes the entry from the directory,
ext4_add_entry() -> __ext4_add_entry() checks:
if (!generic_ci_validate_strict_name(dir, d_name))
return -EINVAL;
Both the primary ext4_add_entry(handle, &new_dentry, inode) and the rollback
ext4_add_entry(handle, &orig_dentry, inode) will then deterministically fail
with -EINVAL, triggering EXT4_ERROR_INODE() and leaving the directory entry
deleted and the target inode orphaned. Should generic_ci_validate_strict_name()
be checked before calling ext4_delete_entry()?
> +
> + if (err) {
> + /* Delete succeeded but re-add failed; try to restore so the
> + * inode is not left without a directory entry. Restore the
> + * original LUFID too: without it the entry is re-added clean
> + * and the old FID-to-path mapping is silently lost. */
> + struct dentry parent_dentry = { .d_inode = dir };
> + struct dentry orig_dentry = {
> + .d_name = *add_name,
> + .d_parent = &parent_dentry,
> + .d_inode = inode,
> + };
> + int rollback_err;
> +
> + if (old_snap.fid) {
> + unsigned int fid_size = old_snap.fid->df_header.ddh_length;
> + struct ext4_dentry_param *old_edp =
> + kmalloc(sizeof(*old_edp) + fid_size, GFP_NOFS);
> +
> + if (old_edp) {
> + old_edp->edp_magic = EXT4_LUFID_MAGIC;
> + memcpy(old_edp->edp_dfid, old_snap.fid, fid_size);
> + orig_dentry.d_fsdata = old_edp;
> + }
> + }
> +
> + rollback_err = ext4_add_entry(handle, &orig_dentry, inode);
> + kfree(orig_dentry.d_fsdata);
> +
> + if (rollback_err)
> + EXT4_ERROR_INODE(dir,
> + "Failed to set LUFID on '%.*s' (err=%d) and failed to restore the original directory entry (err=%d); inode %llu may be orphaned",
> + namelen, filename, err, rollback_err,
> + (unsigned long long)inode->i_ino);
> + goto out_journal;
> + }
[Severity: High]
Why is old_edp allocated with kmalloc(..., GFP_NOFS) after ext4_delete_entry()
has already removed the entry, rather than being pre-allocated before the
deletion like edp in ext4_rename()?
If the primary ext4_add_entry() fails (for example with -ENOSPC, -EDQUOT, or
-ENOMEM) and the GFP_NOFS allocation of old_edp returns NULL,
orig_dentry.d_fsdata remains NULL and the rollback ext4_add_entry() silently
re-adds the entry without its original LUFID.
Also, ext4_dirdata_set_lufid() calls ext4_fname_setup_filename(dir, &d_name, 0,
&fname) at the start of the function, but fname is never used. Instead,
ext4_add_entry() -> __ext4_add_entry() calls ext4_fname_setup_filename() again
under GFP_NOFS during both the re-add and the rollback after the entry has
already been deleted. If the rollback ext4_add_entry() fails (for instance due
to -ENOMEM in ext4_fname_setup_filename()), ext4_journal_stop() still commits
the transaction with the directory entry deleted and without updating
inode->i_nlink or adding inode to the orphan list. Could old_edp and the
filename buffers be prepared before ext4_delete_entry() so rollback does not
depend on post-deletion allocations?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029122413.230409-1-ablagodarenko@thelustrecollective.com?part=11
^ permalink raw reply [flat|nested] 23+ messages in thread