Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3c542d1f authored by Gao Xiang's avatar Gao Xiang Committed by Daniel Rosenberg
Browse files

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 <drosen@google.com>
Cc: Miao Xie <miaoxie@huawei.com>
Cc: Chao Yu <yuchao0@huawei.com>
Change-Id: Ic033e1f84a8b271c1f48010f4e1f189982bbbea2
Signed-off-by: default avatarGao Xiang <gaoxiang25@huawei.com>
Signed-off-by: default avatarDaniel Rosenberg <drosen@google.com>
(cherry picked from commit bd77267426ed5ffe6a25aa77c149cde28f479f95)
parent b21b1272

fs/sdcardfs/inode.c

+3 −0
Original line number Diff line number Diff line
@@ -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;

fs/sdcardfs/lookup.c

+16 −18
Original line number Diff line number Diff line
@@ -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 @@ static struct dentry *__sdcardfs_lookup(struct dentry *dentry, 


	/* 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 @@ static struct dentry *__sdcardfs_lookup(struct dentry *dentry, 
	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);



	/*