From 9597c13b2f3c54240b1b902a677672faa70ab7c5 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Fri, 2 Aug 2013 11:39:32 -0400 Subject: nfs: verify open flags before allowing an atomic open Currently, you can open a NFSv4 file with O_APPEND|O_DIRECT, but cannot fcntl(F_SETFL,...) with those flags. This flag combination is explicitly forbidden on NFSv3 opens, and it seems like it should also be on NFSv4. Reported-by: Chao Ye Signed-off-by: Jeff Layton Signed-off-by: Trond Myklebust --- fs/nfs/dir.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'fs/nfs/dir.c') diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index e474ca2b..39e69d4 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1413,6 +1413,10 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry, dfprintk(VFS, "NFS: atomic_open(%s/%ld), %s\n", dir->i_sb->s_id, dir->i_ino, dentry->d_name.name); + err = nfs_check_flags(open_flags); + if (err) + return err; + /* NFS only supports OPEN on regular files */ if ((open_flags & O_DIRECTORY)) { if (!d_unhashed(dentry)) { -- cgit v1.1 From 5948a401a7f06d67f8548651041e00fd1aafcaf9 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Tue, 20 Aug 2013 12:29:27 -0400 Subject: NFS: Remove the NFSv4 "open optimisation" from nfs_permission Ever since commit 6168f62cb (Add ACCESS operation to OPEN compound) the NFSv4 atomic open has primed the access cache, and so nfs_permission will no longer do an RPC call on the wire. Signed-off-by: Trond Myklebust --- fs/nfs/dir.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'fs/nfs/dir.c') diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 39e69d4..5d737bd 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -2245,11 +2245,6 @@ int nfs_permission(struct inode *inode, int mask) case S_IFLNK: goto out; case S_IFREG: - /* NFSv4 has atomic_open... */ - if (nfs_server_capable(inode, NFS_CAP_ATOMIC_OPEN) - && (mask & MAY_OPEN) - && !(mask & MAY_EXEC)) - goto out; break; case S_IFDIR: /* -- cgit v1.1 From f4ce1299b329e96bb247c95c4fee8809827d6931 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Mon, 19 Aug 2013 18:59:33 -0400 Subject: NFS: Add event tracing for generic NFS events Add tracepoints for inode attribute updates, attribute revalidation, writeback start/end fsync start/end, attribute change start/end, permission check start/end. The intention is to enable performance tracing using 'perf'as well as improving debugging. Signed-off-by: Trond Myklebust --- fs/nfs/dir.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'fs/nfs/dir.c') diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 5d737bd..be3da6f 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -43,6 +43,8 @@ #include "internal.h" #include "fscache.h" +#include "nfstrace.h" + /* #define NFS_DEBUG_VERBOSE 1 */ static int nfs_opendir(struct inode *, struct file *); @@ -2178,9 +2180,11 @@ static int nfs_do_access(struct inode *inode, struct rpc_cred *cred, int mask) struct nfs_access_entry cache; int status; + trace_nfs_access_enter(inode); + status = nfs_access_get_cached(inode, cred, &cache); if (status == 0) - goto out; + goto out_cached; /* Be clever: ask server to check for all possible rights */ cache.mask = MAY_EXEC | MAY_WRITE | MAY_READ; @@ -2193,13 +2197,15 @@ static int nfs_do_access(struct inode *inode, struct rpc_cred *cred, int mask) if (!S_ISDIR(inode->i_mode)) set_bit(NFS_INO_STALE, &NFS_I(inode)->flags); } - return status; + goto out; } nfs_access_add_cache(inode, &cache); +out_cached: + if ((mask & ~cache.mask & (MAY_READ | MAY_WRITE | MAY_EXEC)) != 0) + status = -EACCES; out: - if ((mask & ~cache.mask & (MAY_READ | MAY_WRITE | MAY_EXEC)) == 0) - return 0; - return -EACCES; + trace_nfs_access_exit(inode, status); + return status; } static int nfs_open_permission_mask(int openflags) -- cgit v1.1 From 1472b83eae0bf09ab76ebcb1373dbd210e97f911 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Tue, 20 Aug 2013 11:59:41 -0400 Subject: NFS: Pass in lookup flags from nfs_atomic_open to nfs_lookup When doing an open of a directory, ensure that we do pass the lookup flags from nfs_atomic_open into nfs_lookup. Signed-off-by: Trond Myklebust --- fs/nfs/dir.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'fs/nfs/dir.c') diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index be3da6f..29d5463 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1407,6 +1407,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry, struct dentry *res; struct iattr attr = { .ia_valid = ATTR_OPEN }; struct inode *inode; + unsigned int lookup_flags = 0; int err; /* Expect a negative dentry */ @@ -1429,6 +1430,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry, */ return -ENOENT; } + lookup_flags = LOOKUP_OPEN|LOOKUP_DIRECTORY; goto no_open; } @@ -1479,7 +1481,7 @@ out: return err; no_open: - res = nfs_lookup(dir, dentry, 0); + res = nfs_lookup(dir, dentry, lookup_flags); err = PTR_ERR(res); if (IS_ERR(res)) goto out; -- cgit v1.1 From 6e0d0be715fe041fc7121b0b44cde3015d1cc846 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Tue, 20 Aug 2013 11:26:17 -0400 Subject: NFS: Add event tracing for generic NFS lookups Add tracepoints for lookup, lookup_revalidate and atomic_open Signed-off-by: Trond Myklebust --- fs/nfs/dir.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'fs/nfs/dir.c') diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 29d5463..2263a6b 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1102,7 +1102,9 @@ static int nfs_lookup_revalidate(struct dentry *dentry, unsigned int flags) if (IS_ERR(label)) goto out_error; + trace_nfs_lookup_revalidate_enter(dir, dentry, flags); error = NFS_PROTO(dir)->lookup(dir, &dentry->d_name, fhandle, fattr, label); + trace_nfs_lookup_revalidate_exit(dir, dentry, flags, error); if (error) goto out_bad; if (nfs_compare_fh(NFS_FH(inode), fhandle)) @@ -1315,6 +1317,7 @@ struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry, unsigned in parent = dentry->d_parent; /* Protect against concurrent sillydeletes */ + trace_nfs_lookup_enter(dir, dentry, flags); nfs_block_sillyrename(parent); error = NFS_PROTO(dir)->lookup(dir, &dentry->d_name, fhandle, fattr, label); if (error == -ENOENT) @@ -1341,6 +1344,7 @@ no_entry: nfs_set_verifier(dentry, nfs_save_change_attribute(dir)); out_unblock_sillyrename: nfs_unblock_sillyrename(parent); + trace_nfs_lookup_exit(dir, dentry, flags, error); nfs4_label_free(label); out: nfs_free_fattr(fattr); @@ -1451,12 +1455,14 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry, if (IS_ERR(ctx)) goto out; + trace_nfs_atomic_open_enter(dir, ctx, open_flags); nfs_block_sillyrename(dentry->d_parent); inode = NFS_PROTO(dir)->open_context(dir, ctx, open_flags, &attr); nfs_unblock_sillyrename(dentry->d_parent); if (IS_ERR(inode)) { put_nfs_open_context(ctx); err = PTR_ERR(inode); + trace_nfs_atomic_open_exit(dir, ctx, open_flags, err); switch (err) { case -ENOENT: d_drop(dentry); @@ -1477,6 +1483,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry, } err = nfs_finish_open(ctx, ctx->dentry, file, open_flags, opened); + trace_nfs_atomic_open_exit(dir, ctx, open_flags, err); out: return err; -- cgit v1.1 From 8b0ad3d489cb107804bd8c78695532794eec73d5 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 21 Aug 2013 10:53:09 -0400 Subject: NFS: Add tracepoints for debugging generic file create events Signed-off-by: Trond Myklebust --- fs/nfs/dir.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'fs/nfs/dir.c') diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 2263a6b..9c07812 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1612,7 +1612,9 @@ int nfs_create(struct inode *dir, struct dentry *dentry, attr.ia_mode = mode; attr.ia_valid = ATTR_MODE; + trace_nfs_create_enter(dir, dentry, open_flags); error = NFS_PROTO(dir)->create(dir, dentry, &attr, open_flags); + trace_nfs_create_exit(dir, dentry, open_flags, error); if (error != 0) goto out_err; return 0; -- cgit v1.1 From 1ca42382afd67bf58523d36b00fb4ff487d8173b Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 21 Aug 2013 12:36:04 -0400 Subject: NFS: Add tracepoints for debugging directory changes Add tracepoints for mknod, mkdir, rmdir, remove (unlink) and symlink. Signed-off-by: Trond Myklebust --- fs/nfs/dir.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'fs/nfs/dir.c') diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 9c07812..e41dec5 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1642,7 +1642,9 @@ nfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, dev_t rdev) attr.ia_mode = mode; attr.ia_valid = ATTR_MODE; + trace_nfs_mknod_enter(dir, dentry); status = NFS_PROTO(dir)->mknod(dir, dentry, &attr, rdev); + trace_nfs_mknod_exit(dir, dentry, status); if (status != 0) goto out_err; return 0; @@ -1666,7 +1668,9 @@ int nfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) attr.ia_valid = ATTR_MODE; attr.ia_mode = mode | S_IFDIR; + trace_nfs_mkdir_enter(dir, dentry); error = NFS_PROTO(dir)->mkdir(dir, dentry, &attr); + trace_nfs_mkdir_exit(dir, dentry, error); if (error != 0) goto out_err; return 0; @@ -1689,12 +1693,14 @@ int nfs_rmdir(struct inode *dir, struct dentry *dentry) dfprintk(VFS, "NFS: rmdir(%s/%ld), %s\n", dir->i_sb->s_id, dir->i_ino, dentry->d_name.name); + trace_nfs_rmdir_enter(dir, dentry); error = NFS_PROTO(dir)->rmdir(dir, &dentry->d_name); /* Ensure the VFS deletes this inode */ if (error == 0 && dentry->d_inode != NULL) clear_nlink(dentry->d_inode); else if (error == -ENOENT) nfs_dentry_handle_enoent(dentry); + trace_nfs_rmdir_exit(dir, dentry, error); return error; } @@ -1722,6 +1728,7 @@ static int nfs_safe_remove(struct dentry *dentry) goto out; } + trace_nfs_remove_enter(dir, dentry); if (inode != NULL) { NFS_PROTO(inode)->return_delegation(inode); error = NFS_PROTO(dir)->remove(dir, &dentry->d_name); @@ -1731,6 +1738,7 @@ static int nfs_safe_remove(struct dentry *dentry) error = NFS_PROTO(dir)->remove(dir, &dentry->d_name); if (error == -ENOENT) nfs_dentry_handle_enoent(dentry); + trace_nfs_remove_exit(dir, dentry, error); out: return error; } @@ -1748,13 +1756,14 @@ int nfs_unlink(struct inode *dir, struct dentry *dentry) dfprintk(VFS, "NFS: unlink(%s/%ld, %s)\n", dir->i_sb->s_id, dir->i_ino, dentry->d_name.name); + trace_nfs_unlink_enter(dir, dentry); spin_lock(&dentry->d_lock); if (d_count(dentry) > 1) { spin_unlock(&dentry->d_lock); /* Start asynchronous writeout of the inode */ write_inode_now(dentry->d_inode, 0); error = nfs_sillyrename(dir, dentry); - return error; + goto out; } if (!d_unhashed(dentry)) { __d_drop(dentry); @@ -1766,6 +1775,8 @@ int nfs_unlink(struct inode *dir, struct dentry *dentry) nfs_set_verifier(dentry, nfs_save_change_attribute(dir)); } else if (need_rehash) d_rehash(dentry); +out: + trace_nfs_unlink_exit(dir, dentry, error); return error; } EXPORT_SYMBOL_GPL(nfs_unlink); @@ -1812,7 +1823,9 @@ int nfs_symlink(struct inode *dir, struct dentry *dentry, const char *symname) memset(kaddr + pathlen, 0, PAGE_SIZE - pathlen); kunmap_atomic(kaddr); + trace_nfs_symlink_enter(dir, dentry); error = NFS_PROTO(dir)->symlink(dir, dentry, page, pathlen, &attr); + trace_nfs_symlink_exit(dir, dentry, error); if (error != 0) { dfprintk(VFS, "NFS: symlink(%s/%ld, %s, %s) error %d\n", dir->i_sb->s_id, dir->i_ino, -- cgit v1.1 From 70ded2017072ae16aeaa7fb2a15a879a475161a6 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 21 Aug 2013 12:08:45 -0400 Subject: NFS: Add tracepoints for debugging NFS rename and sillyrename issues Signed-off-by: Trond Myklebust --- fs/nfs/dir.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'fs/nfs/dir.c') diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index e41dec5..dca7deb 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1909,6 +1909,7 @@ int nfs_rename(struct inode *old_dir, struct dentry *old_dentry, new_dentry->d_parent->d_name.name, new_dentry->d_name.name, d_count(new_dentry)); + trace_nfs_rename_enter(old_dir, old_dentry, new_dir, new_dentry); /* * For non-directories, check whether the target is busy and if so, * make a copy of the dentry and then do a silly-rename. If the @@ -1955,6 +1956,8 @@ int nfs_rename(struct inode *old_dir, struct dentry *old_dentry, out: if (rehash) d_rehash(rehash); + trace_nfs_rename_exit(old_dir, old_dentry, + new_dir, new_dentry, error); if (!error) { if (new_inode != NULL) nfs_drop_nlink(new_inode); -- cgit v1.1 From 1fd1085b49f8cafbd0ce4e4682c209a31f7b287f Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 21 Aug 2013 13:54:44 -0400 Subject: NFS: Add tracepoints for debugging NFS hard links Signed-off-by: Trond Myklebust --- fs/nfs/dir.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'fs/nfs/dir.c') diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index dca7deb..4ce7f76 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1860,6 +1860,7 @@ nfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry) old_dentry->d_parent->d_name.name, old_dentry->d_name.name, dentry->d_parent->d_name.name, dentry->d_name.name); + trace_nfs_link_enter(inode, dir, dentry); NFS_PROTO(inode)->return_delegation(inode); d_drop(dentry); @@ -1868,6 +1869,7 @@ nfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry) ihold(inode); d_add(dentry, inode); } + trace_nfs_link_exit(inode, dir, dentry, error); return error; } EXPORT_SYMBOL_GPL(nfs_link); -- cgit v1.1 From 2d9db75005effd6d4e0c8be4f74922e4f413fbe5 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 30 Aug 2013 09:17:33 -0400 Subject: NFS: Fix up two use-after-free issues with the new tracing code We don't want to pass the context argument to trace_nfs_atomic_open_exit() after it has been released. Reported-by: Dan Carpenter Signed-off-by: Trond Myklebust --- fs/nfs/dir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/nfs/dir.c') diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 4ce7f76..d8149e9 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1399,7 +1399,6 @@ static int nfs_finish_open(struct nfs_open_context *ctx, nfs_file_set_open_context(file, ctx); out: - put_nfs_open_context(ctx); return err; } @@ -1460,9 +1459,9 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry, inode = NFS_PROTO(dir)->open_context(dir, ctx, open_flags, &attr); nfs_unblock_sillyrename(dentry->d_parent); if (IS_ERR(inode)) { - put_nfs_open_context(ctx); err = PTR_ERR(inode); trace_nfs_atomic_open_exit(dir, ctx, open_flags, err); + put_nfs_open_context(ctx); switch (err) { case -ENOENT: d_drop(dentry); @@ -1484,6 +1483,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry, err = nfs_finish_open(ctx, ctx->dentry, file, open_flags, opened); trace_nfs_atomic_open_exit(dir, ctx, open_flags, err); + put_nfs_open_context(ctx); out: return err; -- cgit v1.1 From ba6c05928dcafc7e0a0c8e4ee6a293ba47190fd4 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 30 Aug 2013 12:24:25 -0400 Subject: NFS: Ensure that rmdir() waits for sillyrenames to complete If an NFS client does mkdir("dir"); fd = open("dir/file"); unlink("dir/file"); close(fd); rmdir("dir"); then the asynchronous nature of the sillyrename operation means that we can end up getting EBUSY for the rmdir() in the above test. Fix that by ensuring that we wait for any in-progress sillyrenames before sending the rmdir() to the server. Signed-off-by: Trond Myklebust --- fs/nfs/dir.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) (limited to 'fs/nfs/dir.c') diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index d8149e9..187caa4 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1694,12 +1694,19 @@ int nfs_rmdir(struct inode *dir, struct dentry *dentry) dir->i_sb->s_id, dir->i_ino, dentry->d_name.name); trace_nfs_rmdir_enter(dir, dentry); - error = NFS_PROTO(dir)->rmdir(dir, &dentry->d_name); - /* Ensure the VFS deletes this inode */ - if (error == 0 && dentry->d_inode != NULL) - clear_nlink(dentry->d_inode); - else if (error == -ENOENT) - nfs_dentry_handle_enoent(dentry); + if (dentry->d_inode) { + nfs_wait_on_sillyrename(dentry); + error = NFS_PROTO(dir)->rmdir(dir, &dentry->d_name); + /* Ensure the VFS deletes this inode */ + switch (error) { + case 0: + clear_nlink(dentry->d_inode); + break; + case -ENOENT: + nfs_dentry_handle_enoent(dentry); + } + } else + error = NFS_PROTO(dir)->rmdir(dir, &dentry->d_name); trace_nfs_rmdir_exit(dir, dentry, error); return error; -- cgit v1.1