Re: [PATCH v12 00/11] Data in direntry (dirdata) feature

Hi,

  Sashiko flagged 29 potential issues across the v12 series. I have gone
  through all of them.

  The vast majority are false positives. The most common causes:

  - Sashiko reviewed intermediate patch states rather than the final
  committed code (reported build failures and missing updates that do
  not exist in the submitted series).
  - Findings about clearing EXT4_DIRENT_LUFID without compacting
  remaining extensions: ext4_setent_compact_exts() in patch 10 does
  exactly this before the flag is cleared.
  - Several findings concern pre-existing issues in make_indexed_dir(),
  dx_release(), and the htree checksum paths that are not introduced
  by this series.
  - The silent-rename concern for encrypted+casefolded directories in
  SET_LUFID: ext4_fname_setup_filename() normalizes the plaintext
  before encrypting, so different-case input produces the same
  ciphertext.

  If any small issues remain after this analysis they can be addressed
  in the normal review round with developers.

  The series is ready for review. I would appreciate a look from the
  ext4 maintainers.

  Thanks,
  Artem


On Sat, 1 Aug 2026 at 19:13, Artem Blagodarenko
<[email protected]> wrote:
>
> EXT4 currently stores an 8-byte hash in the directory entry immediately
> after the file name to support simultaneous fscrypt and casefold
> functionality.
>
> The dirdata feature provides a general mechanism to store multiple
> metadata records in each directory entry after the NUL filename
> terminator. The unused high 4 bits of 'file_type' indicate which
> records are present; each record starts with a 1-byte length field,
> providing forward compatibility.
>
> The first user of this feature is LUFID (Locally Unique File ID), an
> identifier that must survive rename and be accessible from directory
> readdir without an inode lookup.
>
> e2fsprogs support is provided in a separate patch series.
>
> Thanks to Sashiko AI review for identifying several correctness and
> safety issues in earlier versions of this series.
>
> Changes in v12:
>
> Patch 09 (dirdata LUFID rename): fix late kmalloc(edp) in
> ext4_rename().
>
> When RENAME_WHITEOUT renames a LUFID-carrying entry into a slot that
> does not yet exist (!new.bh), the edp allocation for ext4_add_entry()
> was placed after ext4_setent(&old, ...) had already called
> ext4_setent_compact_exts() to overwrite the LUFID bytes with CFHASH
> data.  An -ENOMEM failure at that point sent the error path through
> ext4_resetent(), which restores the LUFID flag from the saved
> file_type but cannot restore the LUFID bytes =E2=80=94 the same lossy sta=
te
> described in the "Known limitation" comment in the whiteout error
> path, but triggered deterministically by memory pressure rather than
> requiring a journal IO error.
>
> Fix by hoisting the edp allocation before ext4_setent() so that if it
> fails nothing has been modified.  Move edp to function scope and free
> it unconditionally at end_rename.  Update the "Known limitation"
> comment to note that OOM can no longer trigger this path.
>
> Reported-by: sashiko-bot <[email protected]>
> Closes: https://sashiko.dev/#/patchset/20260731235043.44068-1-ablagodaren=
[email protected]?part=3D11
>
> Sashiko v11 findings NOT addressed (false positives):
>
> Part 1 [High] =E2=80=94 OOB read in ext4_htree_next_block when metadata_c=
sum
> is disabled.  Pre-existing, out of scope.
>
> Part 2 [High] =E2=80=94 OOB read when verifying a directory entry near th=
e end
> of a block buffer.  Pre-existing, not introduced by this series.
>
> Part 4 [High] =E2=80=94 get_dtype() broken by high file_type bits.  False
> positive: EXT4_FT_MASK masks the upper bits correctly.
>
> Part 4 [Critical] =E2=80=94 file_type corrupted during VFS rename.  False
> positive: ext4_setent() explicitly manages all dirdata flag transitions
> via masking rather than full overwrite.
>
> Part 5 [Low x2] =E2=80=94 build failures: ext4_dir_entry_len() declaratio=
n and
> dot_de reference in dx_root_limit().  False positives: sashiko reviewed
> a transient version of the code that differs from the submitted patch.
>
> Part 6 [Critical] =E2=80=94 ext4_dir_rec_len() omitting hash space causes
> overflow.  False positive: add_dirent_to_buf() explicitly adds
> sizeof(struct ext4_dirent_hash) to dlen for dirdata+hash_in_dirent;
> including it in both places would double-count.
>
> Part 6 [High x4] =E2=80=94 unvalidated rec_len to ext4_dirent_get_data_le=
n();
> name_len + dirdata integer truncation; length underflow in
> make_indexed_dir() (pre-existing); rename corrupting dirdata flags.
> All false positives: bounds are checked before the calls; truncation
> cannot occur because dirdata is bounded by rec_len; flag management in
> ext4_setent() uses masking.
>
> Part 7 [High] =E2=80=94 OOB read in ext4_read_inline_dir() via new
> ext4_dir_entry_len() call.  False positive: size argument is correctly
> bounded before reaching that path.
>
> Part 7 [Critical] =E2=80=94 double-fetch TOCTOU via DMA mutating de->rec_=
len.
> False positive: DMA-mutating block device threat model does not apply
> to standard ext4 usage.
>
> Part 7 [Low x2] =E2=80=94 build failures from ext4_find_dest_de() prototy=
pe
> change and dx_show_leaf() missed rename.  False positives: series
> builds cleanly; sashiko reviewed different source versions.
>
> Part 8 [Low] =E2=80=94 edp_dfid incompatible pointer type.  Fixed in patc=
h 11
> with explicit cast.
>
> Part 8 [High x3] =E2=80=94 uninitialized stack via fake '.' / '..' entrie=
s
> (pre-existing); missing sizeof(dx_countlimit) in bounds check
> (pre-existing); index corruption for un-migrated entries.  All
> pre-existing or false positives.
>
> Part 9 [High] =E2=80=94 clearing EXT4_DIRENT_LUFID without compacting lea=
ves
> CFHASH at wrong offset.  False positive: ext4_setent_compact_exts()
> physically slides CFHASH bytes into the vacated LUFID slot before
> clearing the flag.
>
> Part 9 [High] =E2=80=94 missing READ_ONCE() on ddh_length in
> ext4_lufid_snapshot().  False positive: dlen is a local unsigned int
> variable; the compiler cannot re-fetch it from the on-disk structure
> after it has been loaded into a register.
>
> Part 10 [High x4] =E2=80=94 uninitialized stack via fake entries (pre-exi=
sting);
> OOB read parsing legacy hashes (pre-existing); memmove() in compact_exts
> skips INO64 between LUFID and CFHASH; ext4_resetent() restores LUFID
> flag but not bytes after compact_exts.
>
> The INO64 concern is a false positive for the current series:
> EXT4_DIRENT_INO64 is defined but no code path in this series writes
> INO64 records to directory entries, so LUFID is always immediately
> followed by CFHASH when both are present.
>
> The resetent finding is the existing "Known limitation" already
> documented in the whiteout error path =E2=80=94 not a new finding.  It is
> addressed in v12 for the OOM-triggered case (see Changes in v12 above);
> the journal-IO-error case remains documented as a known limitation.
>
> Part 11 [High] =E2=80=94 unprivileged user can trigger filesystem read-on=
ly
> remount by inducing memory pressure during SET_LUFID rollback.  Not a
> regression; inherent in the delete-then-re-add approach shared with
> other ext4 operations; per errors=3D mount option behavior is the
> mitigation.
>
> Part 11 [High] =E2=80=94 case-equivalent encrypted filename re-encrypts t=
o
> larger ciphertext.  False positive: fscrypt ciphertext length equals
> plaintext length for all supported modes, so a case-equivalent name
> always produces an identical-length ciphertext.
>
> Artem Blagodarenko (11):
>   ext4: validate count against limit in ext4_dx_csum_verify/_set
>   ext4: replace ext4_dir_entry with ext4_dir_entry_2
>   ext4: add ext4_dir_entry_is_tail()
>   ext4: add dirdata format definitions and access helpers
>   ext4: refactor dx_root to support variable dirent sizes
>   ext4: add ext4_dir_entry_len()
>   ext4: rename ext4_dir_rec_len() and clarify dirdata usage
>   ext4: dirdata feature
>   ext4: add dirdata LUFID support for directory entry rename
>   ext4: add dirdata set/get helpers
>   ext4: Add EXT4_IOC_SET_LUFID ioctl for setting LUFID on directory
>     entries
>
>  fs/ext4/dir.c             |   49 +-
>  fs/ext4/ext4.h            |  245 ++++++-
>  fs/ext4/fast_commit.c     |    1 +
>  fs/ext4/fast_commit.h     |    1 +
>  fs/ext4/inline.c          |   67 +-
>  fs/ext4/ioctl.c           |   85 +++
>  fs/ext4/namei.c           | 1281 +++++++++++++++++++++++++++++++------
>  fs/ext4/sysfs.c           |    2 +
>  include/uapi/linux/ext4.h |   14 +
>  9 files changed, 1495 insertions(+), 250 deletions(-)
>
> --
> 2.43.7