Re: [PATCH 0/3] Data in direntry (dirdata) feature

[tytso@mit.edu (Theodore Tso)]

On Sun, Apr 19, 2026 at 08:37:51PM +0100, Artem Blagodarenko wrote:
> On Sun, 19 Apr 2026 at 01:48, Theodore Tso <tytso@mit.edu> wrote:
> > I'm not seeing that in the patches that was sent out to the list last
> > week.  Where is that?
> 
> I have just sent it to the xfstests mail list and placed you to cc
> 
> > I traced all of the places where ext4_insert_dentry_data() and
> > ext4_dirdata_set() and I don't see *anything* where dirdata was
> > stored, including the fscrypt + casefold hash.   What am I missing?
> 
> dirdata feature should be enabled, and  fscrypt + casefold used
> The new xfstest ext4/065 should help here

Can you show *where* in the patched sources the dirdata gets set in
the fscrypt + casefold case?

Also, there is no real value to users for storing the hash with
fscrypt and casefold using dirdata --- unless they are using dirdata
for some other uses --- but the 64-bit inode number will require
significantly more code changes that's not here.  So there is no
user-visible additional functionality of dirdata.  That's why the
tests that you sent aren't particularly useful.  It doesn't actually
test that the fscrypt hash was stored in dirdata when the dirdata
feature enabled.  It just shows that nothing broke, which is *not* the
same thing.

Another way to demonstrate that that that your tests didn't really
test anything is due to another flaw in your patches.  In addition to
the dirdata feature, there is *also* a dirdata mount option.  If the
dirdata mount option is not specified, then the dirdata flags will get
omitted:

static inline unsigned char get_dtype(struct super_block *sb, int filetype)
{
	unsigned char fl_index = filetype & EXT4_FT_MASK;

	if (!ext4_has_feature_filetype(sb) || fl_index >= EXT4_FT_MAX)
		return DT_UNKNOWN;

	if (!test_opt(sb, DIRDATA))
		return ext4_filetype_table[fl_index];

	return (ext4_filetype_table[fl_index]) |
		(filetype & ~EXT4_FT_MASK);
}

But your proposed new test doesn't *actually* set the dirdata mount
option.  So in addition to my not able to find any place where dirdata
gets *set*, this flaw in your patch (I think it was left over from
when you were using a mount option instead of a file system feature),
meant that nothing could possibly *get* the dirdata flag.  Despite
that, your tests are apparently passing, and you apparently didn't
notice that dirdata support is a no-op.


Finally, please take a look at the KASAN bugs which syzbot found,
which includes some use after frees and out-of-bounds bugs.  (This may
mean that there are some real security bugs in your Luster production
patches that could be found by mechanisms such as Anthropic's Mythos,
so you may want to consider whether the bugs found by Syzkaller might
be applicable in your current product patches.)

https://ci.syzbot.org/series/590e846e-42c0-4497-b6ae-b95ed4468941

Also, if you can rebase your patches onto Linux 7.0 when you send the
next around, then Sashiko will be able to review your patches.

I also suggest that you consider breaking up the patches into smaller
chunks.  This makes them easier to review, and perhaps you would have
noticed that the patch was still defining a worse-than-useless dirdata
mount option.

Finally, perhaps you should consider adding a Kunit test for dirdata?
That would allow you to test the LUFID functionality without needing
to plumb through extra userspace interfaces such as an
EXT4_IOC_SET_LUFID ioctl.

Cheers,

						- Ted