Re: [RFC] ->d_name accesses

[Al Viro <viro@zeniv.linux.org.uk>]

On Fri, Feb 09, 2024 at 11:10:10AM -0800, Nick Desaulniers wrote:
 
> I have 100% observed llvm throw out writes to objects declared as
> const where folks tried to write via "casting away the const" (since
> that's UB) which resulted in boot failures in the Linux kernel
> affecting android devices.  I can't find the commit in question at the
> moment, but seemed to have made some kind of note in it in 2020.
> https://android-review.git.corp.google.com/c/platform/prebuilts/clang/host/linux-x86/+/1201901/1/RELEASE_NOTES.md
> 
> That said, I just tried Al's union, and don't observe such optimizations.
> https://godbolt.org/z/zrj71E8W5

FWIW, see #work.qstr in git://git.kernel.org:/pub/scm/linux/kernel/git/viro/vfs.git

First 4 commits are constifying the arguments that get &dentry->d_inode
passed into, followed by the patch below.  __d_alloc(), swap_names(), copy_name()
and d_mark_tmpfile() - no other writers...

diff --git a/fs/dcache.c b/fs/dcache.c
index b813528fb147..d21df0fb967d 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1651,13 +1651,13 @@ static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
 		dname = dentry->d_iname;
 	}	
 
-	dentry->d_name.len = name->len;
-	dentry->d_name.hash = name->hash;
+	dentry->__d_name.len = name->len;
+	dentry->__d_name.hash = name->hash;
 	memcpy(dname, name->name, name->len);
 	dname[name->len] = 0;
 
 	/* Make sure we always see the terminating NUL character */
-	smp_store_release(&dentry->d_name.name, dname); /* ^^^ */
+	smp_store_release(&dentry->__d_name.name, dname); /* ^^^ */
 
 	dentry->d_lockref.count = 1;
 	dentry->d_flags = 0;
@@ -2695,16 +2695,16 @@ static void swap_names(struct dentry *dentry, struct dentry *target)
 			/*
 			 * Both external: swap the pointers
 			 */
-			swap(target->d_name.name, dentry->d_name.name);
+			swap(target->__d_name.name, dentry->__d_name.name);
 		} else {
 			/*
 			 * dentry:internal, target:external.  Steal target's
 			 * storage and make target internal.
 			 */
-			memcpy(target->d_iname, dentry->d_name.name,
-					dentry->d_name.len + 1);
-			dentry->d_name.name = target->d_name.name;
-			target->d_name.name = target->d_iname;
+			memcpy(target->d_iname, dentry->__d_name.name,
+					dentry->__d_name.len + 1);
+			dentry->__d_name.name = target->__d_name.name;
+			target->__d_name.name = target->d_iname;
 		}
 	} else {
 		if (unlikely(dname_external(dentry))) {
@@ -2712,10 +2712,10 @@ static void swap_names(struct dentry *dentry, struct dentry *target)
 			 * dentry:external, target:internal.  Give dentry's
 			 * storage to target and make dentry internal
 			 */
-			memcpy(dentry->d_iname, target->d_name.name,
-					target->d_name.len + 1);
-			target->d_name.name = dentry->d_name.name;
-			dentry->d_name.name = dentry->d_iname;
+			memcpy(dentry->d_iname, target->__d_name.name,
+					target->__d_name.len + 1);
+			target->__d_name.name = dentry->__d_name.name;
+			dentry->__d_name.name = dentry->d_iname;
 		} else {
 			/*
 			 * Both are internal.
@@ -2728,7 +2728,7 @@ static void swap_names(struct dentry *dentry, struct dentry *target)
 			}
 		}
 	}
-	swap(dentry->d_name.hash_len, target->d_name.hash_len);
+	swap(dentry->__d_name.hash_len, target->__d_name.hash_len);
 }
 
 static void copy_name(struct dentry *dentry, struct dentry *target)
@@ -2738,12 +2738,12 @@ static void copy_name(struct dentry *dentry, struct dentry *target)
 		old_name = external_name(dentry);
 	if (unlikely(dname_external(target))) {
 		atomic_inc(&external_name(target)->u.count);
-		dentry->d_name = target->d_name;
+		dentry->__d_name = target->__d_name;
 	} else {
-		memcpy(dentry->d_iname, target->d_name.name,
-				target->d_name.len + 1);
-		dentry->d_name.name = dentry->d_iname;
-		dentry->d_name.hash_len = target->d_name.hash_len;
+		memcpy(dentry->d_iname, target->__d_name.name,
+				target->__d_name.len + 1);
+		dentry->__d_name.name = dentry->d_iname;
+		dentry->__d_name.hash_len = target->__d_name.hash_len;
 	}
 	if (old_name && likely(atomic_dec_and_test(&old_name->u.count)))
 		kfree_rcu(old_name, u.head);
@@ -3080,7 +3080,7 @@ void d_mark_tmpfile(struct file *file, struct inode *inode)
 		!d_unlinked(dentry));
 	spin_lock(&dentry->d_parent->d_lock);
 	spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
-	dentry->d_name.len = sprintf(dentry->d_iname, "#%llu",
+	dentry->__d_name.len = sprintf(dentry->d_iname, "#%llu",
 				(unsigned long long)inode->i_ino);
 	spin_unlock(&dentry->d_lock);
 	spin_unlock(&dentry->d_parent->d_lock);
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index eb4ca8a1d948..a74370b1fe31 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -85,7 +85,10 @@ struct dentry {
 	seqcount_spinlock_t d_seq;	/* per dentry seqlock */
 	struct hlist_bl_node d_hash;	/* lookup hash list */
 	struct dentry *d_parent;	/* parent directory */
-	struct qstr d_name;
+	union {
+	struct qstr __d_name;
+	const struct qstr d_name;
+	};
 	struct inode *d_inode;		/* Where the name belongs to - NULL is
 					 * negative */
 	unsigned char d_iname[DNAME_INLINE_LEN];	/* small names */