libfs: fix get_stashed_dentry()

Commit Message

Christian Brauner Sept. 6, 2024, 4:22 p.m. UTC

get_stashed_dentry() tries to optimistically retrieve a stashed dentry
from a provided location. It needs to ensure to hold rcu lock before it
dereference the stashed location to prevent UAF issues. Use
rcu_dereference() instead of READ_ONCE() it's effectively equivalent
with some lockdep bells and whistles and it communicates clearly that
this expects rcu protection.

Link: https://lore.kernel.org/r/20240906-vfs-hotfix-5959800ffa68@brauner
Fixes: 07fd7c329839 ("libfs: add path_from_stashed()")
Reported-by: [e-mail]
Fixes: [e-mail]
Reported-by: [e-mail]
Fixes: [e-mail]
Signed-off-by: Christian Brauner <[e-mail]>
---
Hey Linus,

Would you mind applying this fix directly? I should fix two syzbot
reports. Apparently that was already detected in June but not reported
due to a missing reproducer. I reckon it's pretty difficult to get a
reliable reproducer for this issue.

Thanks!
Christian
---
 fs/libfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Linus Torvalds Sept. 6, 2024, 6:09 p.m. UTC | #1

On Fri, 6 Sept 2024 at 09:24, Christian Brauner <[e-mail]> wrote:
>
> Would you mind applying this fix directly?

Applied.

                Linus

Patch

diff --git a/fs/libfs.c b/fs/libfs.c
index 7874b23364e1..0e1b99923802 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -2125,12 +2125,12 @@  struct timespec64 simple_inode_init_ts(struct inode *inode)
 }
 EXPORT_SYMBOL(simple_inode_init_ts);
 
-static inline struct dentry *get_stashed_dentry(struct dentry *stashed)
+static inline struct dentry *get_stashed_dentry(struct dentry **stashed)
 {
 	struct dentry *dentry;
 
 	guard(rcu)();
-	dentry = READ_ONCE(stashed);
+	dentry = rcu_dereference(*stashed);
 	if (!dentry)
 		return NULL;
 	if (!lockref_get_not_dead(&dentry->d_lockref))
@@ -2227,7 +2227,7 @@  int path_from_stashed(struct dentry **stashed, struct vfsmount *mnt, void *data,
 	const struct stashed_operations *sops = mnt->mnt_sb->s_fs_info;
 
 	/* See if dentry can be reused. */
-	path->dentry = get_stashed_dentry(*stashed);
+	path->dentry = get_stashed_dentry(stashed);
 	if (path->dentry) {
 		sops->put_data(data);
 		goto out_path;