linux-kernel - [PATCH RFC v3] vfs: make fstatat retry once on ESTALE errors from getattr call

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1334749927-26138-1-git-send-email-jlayton@redhat.com>
Date:	Wed, 18 Apr 2012 07:52:07 -0400
From:	Jeff Layton <jlayton@...hat.com>
To:	linux-fsdevel@...r.kernel.org, linux-nfs@...r.kernel.org,
	linux-kernel@...r.kernel.org
Cc:	miklos@...redi.hu, viro@...IV.linux.org.uk, hch@...radead.org,
	michael.brantley@...haw.com, sven.breuner@...m.fraunhofer.de,
	chuck.lever@...cle.com, pstaubach@...grid.com, malahal@...ibm.com,
	bfields@...ldses.org, trond.myklebust@....uio.no, rees@...ch.edu
Subject: [PATCH RFC v3] vfs: make fstatat retry once on ESTALE errors from getattr call

ESTALE errors are a source of pain for many users of NFS. Usually they
occur when a file is removed from the server after a successful lookup
against it.

Luckily, the remedy in these cases is usually simple. We should just
redo the lookup, forcing revalidations all the way in and then retry the
call. We of course cannot do this for syscalls that do not involve a
path, but for path-based syscalls we can and should attempt to recover
from an ESTALE.

This patch implements this by having the VFS reattempt the lookup (with
LOOKUP_REVAL set) and call exactly once when it would ordinarily return
ESTALE. This should catch the bulk of these cases under normal usage,
without unduly inconveniencing other filesystems that return ESTALE on
path-based syscalls.

Note that it's possible to hit this race more than once, but a single
retry should catch the bulk of these cases under normal circumstances.

This patch is just an example. We'll alter most path-based syscalls in a
similar fashion to fix this correctly. At this point, I'm just trying to
ensure that the retry semantics are acceptable before I being that work.

Does anyone have strong objections to this patch? I'm aware that the
retry mechanism is not as robust as many (e.g. Peter) would like, but it
should at least improve the current situation.

If no one has a strong objection, then I'll start going through and
adding similar code to the other syscalls. And we can hopefully we can
get at least some of them in for 3.5.

Signed-off-by: Jeff Layton <jlayton@...hat.com>
---
 fs/stat.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/fs/stat.c b/fs/stat.c
index c733dc5..0ee9cb4 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -73,7 +73,8 @@ int vfs_fstatat(int dfd, const char __user *filename, struct kstat *stat,
 {
 	struct path path;
 	int error = -EINVAL;
-	int lookup_flags = 0;
+	bool retried = false;
+	unsigned int lookup_flags = 0;

 	if ((flag & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT |
 		      AT_EMPTY_PATH)) != 0)
@@ -84,12 +85,18 @@ int vfs_fstatat(int dfd, const char __user *filename, struct kstat *stat,
 	if (flag & AT_EMPTY_PATH)
 		lookup_flags |= LOOKUP_EMPTY;

+retry:
 	error = user_path_at(dfd, filename, lookup_flags, &path);
 	if (error)
 		goto out;

 	error = vfs_getattr(path.mnt, path.dentry, stat);
 	path_put(&path);
+	if (error == -ESTALE && !retried) {
+		retried = true;
+		lookup_flags |= LOOKUP_REVAL;
+		goto retry;
+	}
 out:
 	return error;
 }
-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/