Question about possible use-after-free in nfs_direct_write_reschedule()

["NeilBrown" <neilb@xxxxxxx>]

I've been reviewing some nfs/direct.c patches for possible backport to
one of our older enterprise kernels and I've found something that looks
wrong.

It isn't clear to me how to exercise the code so I haven't be able to
trigger a problem.  I'm hoping that someone could either explain when
this code runs, or confirm if the code is correct or not.

Commit 954998b60caa ("NFS: Fix error handling for O_DIRECT write scheduling")

adds an extra call to nfs_release_request() but I cannot find any place
that an extra reference is taken.

The code currently reads:

	while (!list_empty(&reqs)) {
		req = nfs_list_entry(reqs.next);
		nfs_list_remove_request(req);
		nfs_unlock_and_release_request(req);
		if (desc.pg_error == -EAGAIN) {
			nfs_mark_request_commit(req, NULL, &cinfo, 0);
		} else {
			spin_lock(&dreq->lock);
			nfs_direct_truncate_request(dreq, req);
			spin_unlock(&dreq->lock);
			nfs_release_request(req);
		}
	}

after the nfs_unlock_and_release_request() call I would expect that the
request could be freed, so that nfs_mark_request_commit() or the
nfs_release_request() could cause a problem.

Superficially it looks like the call should be simply
nfs_unlock_request().  This would follow the
list_remove;unlock;mark_commit pattern also found in
nfs_direct_write_reschedule_io().

Do we need:
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -581,7 +581,7 @@ static void nfs_direct_write_reschedule(struct nfs_direct_req *dreq)
 	while (!list_empty(&reqs)) {
 		req = nfs_list_entry(reqs.next);
 		nfs_list_remove_request(req);
-		nfs_unlock_and_release_request(req);
+		nfs_unlock_request(req);
 		if (desc.pg_error == -EAGAIN) {
 			nfs_mark_request_commit(req, NULL, &cinfo, 0);
 		} else {

??

Thanks,
NeilBrown