linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] d_move() vs d_unhashed() race: retry under d_lock
@ 2017-09-08 16:21 Goldwyn Rodrigues
  2017-09-09  0:14 ` NeilBrown
  0 siblings, 1 reply; 2+ messages in thread
From: Goldwyn Rodrigues @ 2017-09-08 16:21 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: viro, alexey.lyashkov, neilb, Goldwyn Rodrigues

From: Goldwyn Rodrigues <rgoldwyn@suse.com>

This is a follow-up of Alexey's patch at
https://patchwork.kernel.org/patch/9455345/
with suggestions proposed by Al Viro.

d_move() and d_unhashed() may race because there is a small window
where the dentry is unhashed. This may result in ENOENT (for getcwd).
This must be checked under d_lock. However, in order to keep the fast
path, perform the d_unhashed without d_lock first, and in the unlikely
event that it succeeds, perform the check again under d_lock.

Here is the test case which demonstrates the problem:

void *thread_main(void *unused)
{
	int rc;

	for (;;) {
		rc = rename("/tmp/t-a", "/tmp/t-b");
		assert(rc == 0);
		rc = rename("/tmp/t-b", "/tmp/t-a");
		assert(rc == 0);
	}

	return NULL;
}

int main(void)
{
	int rc, i;
	pthread_t ptt;

	rmdir("/tmp/t-a");
	rmdir("/tmp/t-b");

	rc = mkdir("/tmp/t-a", 0666);
	assert(rc == 0);

	rc = chdir("/tmp/t-a");
	assert(rc == 0);

	rc = pthread_create(&ptt, NULL, thread_main, NULL);
	assert(rc == 0);

	for (i = 0;; i++) {
		char buf[100], *b;
		b = getcwd(buf, sizeof(buf));
		if (b == NULL) {
			printf("getcwd failed on iter %d with %d\n", i,
					errno);
			break;
		}
	}

	return 0;
}

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/dcache.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index f90141387f01..ebda7e20ae86 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -3224,6 +3224,18 @@ char *d_absolute_path(const struct path *path,
 	return res;
 }
 
+static inline bool d_unhashed_safe(struct dentry *dentry)
+{
+        bool ret = d_unhashed(dentry);
+        if (unlikely(ret)) {
+                /* retry under d_lock */
+                spin_lock(&dentry->d_lock);
+                ret = d_unhashed(dentry);
+                spin_unlock(&dentry->d_lock);
+        }
+        return ret;
+}
+
 /*
  * same as __d_path but appends "(deleted)" for unlinked files.
  */
@@ -3232,7 +3244,7 @@ static int path_with_deleted(const struct path *path,
 			     char **buf, int *buflen)
 {
 	prepend(buf, buflen, "\0", 1);
-	if (d_unlinked(path->dentry)) {
+	if (d_unhashed_safe(path->dentry)) {
 		int error = prepend(buf, buflen, " (deleted)", 10);
 		if (error)
 			return error;
@@ -3396,7 +3408,7 @@ char *dentry_path(struct dentry *dentry, char *buf, int buflen)
 	char *p = NULL;
 	char *retval;
 
-	if (d_unlinked(dentry)) {
+	if (d_unhashed_safe(dentry)) {
 		p = buf + buflen;
 		if (prepend(&p, &buflen, "//deleted", 10) != 0)
 			goto Elong;
@@ -3453,7 +3465,7 @@ SYSCALL_DEFINE2(getcwd, char __user *, buf, unsigned long, size)
 	get_fs_root_and_pwd_rcu(current->fs, &root, &pwd);
 
 	error = -ENOENT;
-	if (!d_unlinked(pwd.dentry)) {
+	if (!d_unhashed_safe(pwd.dentry)) {
 		unsigned long len;
 		char *cwd = page + PATH_MAX;
 		int buflen = PATH_MAX;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-09-09  0:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-08 16:21 [PATCH] d_move() vs d_unhashed() race: retry under d_lock Goldwyn Rodrigues
2017-09-09  0:14 ` NeilBrown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).