2024-03-01 17:07:03

by Josef Bacik

Subject: [PATCH 0/2] nfs: fix UAF in direct writes

Hello,

Here are two patches to address a UAF we've been seeing in our stress testing
with NFS internally. The first one is to clean up the accesses to the
nfs_direct_req fields which is more a matter of correctness than an actual
observed problem. The second patch is to address the problem itself. I've been
running these patches on my stress test for a few hours now, I would appreciate
feedback and review to see if this is the correct way to fix the problem.
Thanks,

Josef

Josef Bacik (2):
nfs: properly protect nfs_direct_req fields
nfs: fix UAF in direct writes

fs/nfs/direct.c | 18 +++++++++++++++---
fs/nfs/write.c | 2 +-
include/linux/nfs_fs.h | 1 +
3 files changed, 17 insertions(+), 4 deletions(-)

--
2.43.0

2024-03-01 17:33:14

by Josef Bacik

Subject: [PATCH 1/2] nfs: properly protect nfs_direct_req fields

We protect accesses to the nfs_direct_req fields with the dreq->lock
ever where except nfs_direct_commit_complete. This isn't a huge deal,
but it does lead to confusion, and we could potentially end up setting
NFS_ODIRECT_RESCHED_WRITES in one thread where we've had an error in
another. Clean this up to properly protect ->error and ->flags in the
commit completion path.

Signed-off-by: Josef Bacik <[email protected]>
---
fs/nfs/direct.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index c03926a1cc73..befcc167e25f 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -606,6 +606,7 @@ static void nfs_direct_commit_complete(struct nfs_commit_data *data)

trace_nfs_direct_commit_complete(dreq);

+ spin_lock(&dreq->lock);
if (status < 0) {
/* Errors in commit are fatal */
dreq->error = status;
@@ -613,6 +614,7 @@ static void nfs_direct_commit_complete(struct nfs_commit_data *data)
} else {
status = dreq->error;
}
+ spin_unlock(&dreq->lock);

nfs_init_cinfo_from_dreq(&cinfo, dreq);

@@ -625,7 +627,10 @@ static void nfs_direct_commit_complete(struct nfs_commit_data *data)
spin_unlock(&dreq->lock);
nfs_release_request(req);
} else if (!nfs_write_match_verf(verf, req)) {
- dreq->flags = NFS_ODIRECT_RESCHED_WRITES;
+ spin_lock(&dreq->lock);
+ if (dreq->flags == 0)
+ dreq->flags = NFS_ODIRECT_RESCHED_WRITES;
+ spin_unlock(&dreq->lock);
/*
* Despite the reboot, the write was successful,
* so reset wb_nio.
--
2.43.0