* [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; 3+ 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] 3+ 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; 3+ 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] 3+ 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; 3+ 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] 3+ messages in thread
end of thread, other threads:[~2019-05-13 15:27 UTC | newest]
Thread overview: 3+ 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
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).