* [PATCH 0/2] nfsd shoudn't call mnt_want_write twice
@ 2019-05-13 15:27 J. Bruce Fields
2019-05-13 15:27 ` [PATCH 1/2] nfsd: allow fh_want_write to be called twice J. Bruce Fields
2019-05-13 15:27 ` [PATCH 2/2] nfsd: fh_drop_write in nfsd_unlink J. Bruce Fields
0 siblings, 2 replies; 4+ messages in thread
From: J. Bruce Fields @ 2019-05-13 15:27 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
A fuzzer recently triggered lockdep warnings about potential sb_writers
deadlocks caused by fh_want_write, showing that we haven't been careful
to pair each fh_want_write() with an fh_drop_write().
This isn't normally a problem since fh_put() will call fh_drop_write()
for us. And that's OK for NFSv3. But an NFSv4 protocol fuzzer can do
weird things like call unlink twice in a compound.
So we can either make it safe to call fh_want_write() twice, or we can
fix all the callers to call fh_drop_write().
For now I think we have to do the former just to get the bug fixed.
Long term I don't know whether it's best to stay with that or to fix up
the callers. I fixed nfsd_unlink just because it's easy, but maybe
that's pointless, it's others (like setattr) that are the complicated
ones.
J. Bruce Fields (2):
nfsd: allow fh_want_write to be called twice
nfsd: fh_drop_write in nfsd_unlink
fs/nfsd/vfs.c | 8 +++++---
fs/nfsd/vfs.h | 5 ++++-
2 files changed, 9 insertions(+), 4 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] nfsd: allow fh_want_write to be called twice
2019-05-13 15:27 [PATCH 0/2] nfsd shoudn't call mnt_want_write twice J. Bruce Fields
@ 2019-05-13 15:27 ` J. Bruce Fields
2019-05-13 15:27 ` [PATCH 2/2] nfsd: fh_drop_write in nfsd_unlink J. Bruce Fields
1 sibling, 0 replies; 4+ messages in thread
From: J. Bruce Fields @ 2019-05-13 15:27 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
A fuzzer recently triggered lockdep warnings about potential sb_writers
deadlocks caused by fh_want_write().
Looks like we aren't careful to pair each fh_want_write() with an
fh_drop_write().
It's not normally a problem since fh_put() will call fh_drop_write() for
us. And was OK for NFSv3 where we'd do one operation that might call
fh_want_write(), and then put the filehandle.
But an NFSv4 protocol fuzzer can do weird things like call unlink twice
in a compound, and then we get into trouble.
I'm a little worried about this approach of just leaving everything to
fh_put(). But I think there are probably a lot of
fh_want_write()/fh_drop_write() imbalances so for now I think we need it
to be more forgiving.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/vfs.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index a7e107309f76..db351247892d 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -120,8 +120,11 @@ void nfsd_put_raparams(struct file *file, struct raparms *ra);
static inline int fh_want_write(struct svc_fh *fh)
{
- int ret = mnt_want_write(fh->fh_export->ex_path.mnt);
+ int ret;
+ if (fh->fh_want_write)
+ return 0;
+ ret = mnt_want_write(fh->fh_export->ex_path.mnt);
if (!ret)
fh->fh_want_write = true;
return ret;
--
2.21.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] nfsd: fh_drop_write in nfsd_unlink
2019-05-13 15:27 [PATCH 0/2] nfsd shoudn't call mnt_want_write twice J. Bruce Fields
2019-05-13 15:27 ` [PATCH 1/2] nfsd: allow fh_want_write to be called twice J. Bruce Fields
@ 2019-05-13 15:27 ` J. Bruce Fields
1 sibling, 0 replies; 4+ messages in thread
From: J. Bruce Fields @ 2019-05-13 15:27 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
fh_want_write() can now be called twice, but I'm also fixing up the
callers not to do that.
Other cases include setattr and create.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/vfs.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 7dc98e14655d..fc24ee47eab5 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1786,12 +1786,12 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
rdentry = lookup_one_len(fname, dentry, flen);
host_err = PTR_ERR(rdentry);
if (IS_ERR(rdentry))
- goto out_nfserr;
+ goto out_drop_write;
if (d_really_is_negative(rdentry)) {
dput(rdentry);
- err = nfserr_noent;
- goto out;
+ host_err = -ENOENT;
+ goto out_drop_write;
}
if (!type)
@@ -1805,6 +1805,8 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
host_err = commit_metadata(fhp);
dput(rdentry);
+out_drop_write:
+ fh_drop_write(fhp);
out_nfserr:
err = nfserrno(host_err);
out:
--
2.21.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 00/12] exposing knfsd state to userspace
@ 2019-05-16 1:20 J. Bruce Fields
2019-05-16 1:20 ` [PATCH 2/2] nfsd: fh_drop_write in nfsd_unlink J. Bruce Fields
0 siblings, 1 reply; 4+ messages in thread
From: J. Bruce Fields @ 2019-05-16 1:20 UTC (permalink / raw)
To: linux-nfs; +Cc: linux-fsdevel, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
This is still a little rough, but maybe closer. Changes since last
time:
- renamed the "opens" file to "states" and added some (minimal)
information about lock, delegation, and layout stateids as
well as opens.
- converted the states file to a YAML-like format.
- added the ability to remove a client's state by writing
"expire\n" into a new nfsd/client/#/ctl file.
Recapping discussion from last time:
The following patches expose information about NFSv4 state held by knfsd
on behalf of NFSv4 clients. This especially important for opens, which
are currently invisible to userspace on the server, unlike locks
(/proc/locks) and local processes' opens (under /proc/<pid>/).
The approach is to add a new directory /proc/fs/nfsd/clients/ with
subdirectories for each active NFSv4 client. Each subdirectory has an
"info" file with some basic information to help identify the client and
a "states" directory that lists the open state held by that client.
Currently these pseudofiles look like:
# find /proc/fs/nfsd/clients -type f|xargs tail
==> /proc/fs/nfsd/clients/3/opens <==
- 0x020000006a5fdc5c4ad09d9e01000000: { type: open, access: rw, deny: --, superblock: "fd:10:13649", owner: "open id:\x00\x00\x00&\x00\x00\x00\x00\x00\x0046��QH " }
- 0x010000006a5fdc5c4ad09d9e03000000: { type: open, access: r-, deny: --, superblock: "fd:10:13650", owner: "open id:\x00\x00\x00&\x00\x00\x00\x00\x00\x0046��QH" }
- 0x010000006a5fdc5c4ad09d9e04000000: { type: deleg, access: r, superblock: "fd:10:13650" }
- 0x010000006a5fdc5c4ad09d9e06000000: { type: lock, superblock: "fd:10:13649", owner: "lock id:\x00\x00\x00&\x00\x00\x00\x00\x00\x00\x00\x00" }
==> /proc/fs/nfsd/clients/3/info <==
clientid: 6debfb505cc0cd36
address: 192.168.122.36:907
name: "Linux NFSv4.2 test2.fieldses.org"
minor version: 2
I'm conflicted about how I'm representing stateowners and client names,
both opaque byte-streams in the protocol but that often include
human-readable ascii.
Possibly also todo:
- add some information about krb5 principals to the clients
file.
- add information about the stateids used to represent
asynchronous copies. They're a little different from the
other stateids and might end up in a separate "copies" file,
- this duplicates some functionality of the little-used fault
injection code; could we replace it entirely?
- some of the bits of filesystem code could probably be shared
with rpc_pipefs and libfs.
--b.
J. Bruce Fields (10):
nfsd: persist nfsd filesystem across mounts
nfsd: rename cl_refcount
nfsd4: use reference count to free client
nfsd: add nfsd/clients directory
nfsd: make client/ directory names small ints
rpc: replace rpc_filelist by tree_descr
nfsd4: add a client info file
nfsd4: add file to display list of client's opens
nfsd: expose some more information about NFSv4 opens
nfsd: add more information to client info file
fs/nfsd/netns.h | 6 +
fs/nfsd/nfs4state.c | 228 ++++++++++++++++++++++++++++++---
fs/nfsd/nfsctl.c | 225 +++++++++++++++++++++++++++++++-
fs/nfsd/nfsd.h | 11 ++
fs/nfsd/state.h | 9 +-
fs/seq_file.c | 17 +++
include/linux/seq_file.h | 2 +
include/linux/string_helpers.h | 1 +
lib/string_helpers.c | 5 +-
net/sunrpc/rpc_pipe.c | 37 ++----
10 files changed, 491 insertions(+), 50 deletions(-)
--
2.20.1
J. Bruce Fields (12):
nfsd: persist nfsd filesystem across mounts
nfsd: rename cl_refcount
nfsd4: use reference count to free client
nfsd: add nfsd/clients directory
nfsd: make client/ directory names small ints
nfsd4: add a client info file
nfsd: copy client's address including port number to cl_addr
nfsd: add more information to client info file
nfsd4: add file to display list of client's opens
nfsd: show lock and deleg stateids
nfsd4: show layout stateids
nfsd: allow forced expiration of NFSv4 clients
fs/nfsd/netns.h | 6 +
fs/nfsd/nfs4state.c | 418 ++++++++++++++++++++++++++++++++++++---
fs/nfsd/nfsctl.c | 225 ++++++++++++++++++++-
fs/nfsd/nfsd.h | 11 ++
fs/nfsd/state.h | 7 +-
fs/seq_file.c | 17 ++
include/linux/seq_file.h | 2 +
7 files changed, 661 insertions(+), 25 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] nfsd: fh_drop_write in nfsd_unlink
2019-05-16 1:20 [PATCH 00/12] exposing knfsd state to userspace J. Bruce Fields
@ 2019-05-16 1:20 ` J. Bruce Fields
0 siblings, 0 replies; 4+ messages in thread
From: J. Bruce Fields @ 2019-05-16 1:20 UTC (permalink / raw)
To: linux-nfs; +Cc: linux-fsdevel, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
fh_want_write() can now be called twice, but I'm also fixing up the
callers not to do that.
Other cases include setattr and create.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/vfs.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 7dc98e14655d..fc24ee47eab5 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1786,12 +1786,12 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
rdentry = lookup_one_len(fname, dentry, flen);
host_err = PTR_ERR(rdentry);
if (IS_ERR(rdentry))
- goto out_nfserr;
+ goto out_drop_write;
if (d_really_is_negative(rdentry)) {
dput(rdentry);
- err = nfserr_noent;
- goto out;
+ host_err = -ENOENT;
+ goto out_drop_write;
}
if (!type)
@@ -1805,6 +1805,8 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
host_err = commit_metadata(fhp);
dput(rdentry);
+out_drop_write:
+ fh_drop_write(fhp);
out_nfserr:
err = nfserrno(host_err);
out:
--
2.21.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-16 1:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-13 15:27 [PATCH 0/2] nfsd shoudn't call mnt_want_write twice J. Bruce Fields
2019-05-13 15:27 ` [PATCH 1/2] nfsd: allow fh_want_write to be called twice J. Bruce Fields
2019-05-13 15:27 ` [PATCH 2/2] nfsd: fh_drop_write in nfsd_unlink J. Bruce Fields
2019-05-16 1:20 [PATCH 00/12] exposing knfsd state to userspace J. Bruce Fields
2019-05-16 1:20 ` [PATCH 2/2] nfsd: fh_drop_write in nfsd_unlink J. Bruce Fields
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).