linux-kernel - Re: [git pull] d

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250128002659.GJ1977892@ZenIV>
Date: Tue, 28 Jan 2025 00:26:59 +0000
From: Al Viro <viro@...iv.linux.org.uk>
To: Sasha Levin <sashal@...nel.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [git pull] d_revalidate pile

On Mon, Jan 27, 2025 at 06:26:58PM -0500, Sasha Levin wrote:
> On Mon, Jan 27, 2025 at 10:40:59PM +0000, Al Viro wrote:
> > On Mon, Jan 27, 2025 at 09:34:56PM +0000, Al Viro wrote:
> > 
> > > If so, then
> > > 	a) it's a false positive (and IIRC, it's not the first time
> > > kfence gets confused by that)
> > > 	b) your bisection will probably converge to bdd9951f60f9
> > > "dissolve external_name.u into separate members" which is where we'd
> > > ended up with offsetof(struct external_name, name) being 4 modulo 8.
> > > 
> > > As a quick test, try to flip the order of head and count in
> > > struct external_name and see if that makes the warning go away.
> > > If it does, I'm pretty certain that theory above is correct.
> > 
> > Not quite...   dentry_string_cmp() assumes that ->d_name.name is
> > word-aligned, so load_unaligned_zeropad() is done only to the
> > second string (the one we compare against).
> 
> Sorry for the silence on my end: this issue doesn't reproduce
> consistently, so I need to do more runs for these tests.

Updated version force-pushed; delta is

diff --git a/fs/dcache.c b/fs/dcache.c
index 695406e48937..903142b324e9 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -295,10 +295,16 @@ static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *c
 	return dentry_string_cmp(cs, ct, tcount);
 }
 
+/*
+ * long names are allocated separately from dentry and never modified.
+ * Refcounted, freeing is RCU-delayed.  See take_dentry_name_snapshot()
+ * for the reason why ->count and ->head can't be combined into a union.
+ * dentry_string_cmp() relies upon ->name[] being word-aligned.
+ */
 struct external_name {
-	struct rcu_head head;	// ->head and ->count can't be combined
-	atomic_t count;		// see take_dentry_name_snapshot()
-	unsigned char name[];
+	atomic_t count;
+	struct rcu_head head;
+	unsigned char name[] __aligned(sizeof(unsigned long));
 };
 
 static inline struct external_name *external_name(struct dentry *dentry)


Could you recheck that one (23e8b451dea4)?  I'll send an update pull request
if nothing wrong shows up.