From 680425be3514e90bd4bd7e34cd68633496027071 Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Mon, 13 Aug 2018 09:56:43 +0800 Subject: [PATCH] ANDROID: sdcardfs: fix -ENOENT lookup race issue The negative lower dentry created by vfs_path_lookup could be reclaimed between vfs_path_lookup and d_hash_and_lookup. Therefore, it is unsafe to just lookup dcache again for the negative dentry cases. Without this patch, users could occasionally get trapped into `failed to create' under memory pressure. So here is a workaround to hack it and in my opinion sdcardfs should be refactored to close all races in the long term as pointed out in the code comment of this commit. Test: (Thread 1) while true; do echo 3 > /proc/sys/vm/drop_caches done (Thread 2) i=0 while true; do echo 123 > /sdcard/$i i=$((i+1)) done Bug: 63872684 Cc: Daniel Rosenberg Cc: Miao Xie Cc: Chao Yu Change-Id: Ic033e1f84a8b271c1f48010f4e1f189982bbbea2 Signed-off-by: Gao Xiang Signed-off-by: Daniel Rosenberg (cherry picked from commit bd77267426ed5ffe6a25aa77c149cde28f479f95) --- fs/sdcardfs/inode.c | 3 +++ fs/sdcardfs/lookup.c | 34 ++++++++++++++++------------------ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/fs/sdcardfs/inode.c b/fs/sdcardfs/inode.c index 4dd681e0d59d..edeca118cce5 100644 --- a/fs/sdcardfs/inode.c +++ b/fs/sdcardfs/inode.c @@ -87,6 +87,9 @@ static int sdcardfs_create(struct inode *dir, struct dentry *dentry, lower_dentry_mnt = lower_path.mnt; lower_parent_dentry = lock_parent(lower_dentry); + if (d_is_positive(lower_dentry)) + return -EEXIST; + /* set last 16bytes of mode field to 0664 */ mode = (mode & S_IFMT) | 00664; diff --git a/fs/sdcardfs/lookup.c b/fs/sdcardfs/lookup.c index a5c9686090e0..d2dfdf1028c6 100644 --- a/fs/sdcardfs/lookup.c +++ b/fs/sdcardfs/lookup.c @@ -257,7 +257,6 @@ static struct dentry *__sdcardfs_lookup(struct dentry *dentry, struct dentry *lower_dentry; const struct qstr *name; struct path lower_path; - struct qstr dname; struct dentry *ret_dentry = NULL; struct sdcardfs_sb_info *sbi; @@ -316,6 +315,7 @@ put_name: /* no error: handle positive dentries */ if (!err) { +found: /* check if the dentry is an obb dentry * if true, the lower_inode must be replaced with * the inode of the graft path @@ -362,28 +362,26 @@ put_name: if (err && err != -ENOENT) goto out; - /* instatiate a new negative dentry */ - dname.name = name->name; - dname.len = name->len; - - /* See if the low-level filesystem might want - * to use its own hash - */ - lower_dentry = d_hash_and_lookup(lower_dir_dentry, &dname); - if (IS_ERR(lower_dentry)) - return lower_dentry; - - if (!lower_dentry) { - /* We called vfs_path_lookup earlier, and did not get a negative - * dentry then. Don't confuse the lower filesystem by forcing - * one on it now... - */ - err = -ENOENT; + /* get a (very likely) new negative dentry */ + lower_dentry = lookup_one_len_unlocked(name->name, + lower_dir_dentry, name->len); + if (IS_ERR(lower_dentry)) { + err = PTR_ERR(lower_dentry); goto out; } lower_path.dentry = lower_dentry; lower_path.mnt = mntget(lower_dir_mnt); + + /* + * Check if someone sneakily filled in the dentry when + * we weren't looking. We'll check again in create. + */ + if (unlikely(d_inode_rcu(lower_dentry))) { + err = 0; + goto found; + } + sdcardfs_set_lower_path(dentry, &lower_path); /*