Commit 7fc0564e authored by Andrew Elble's avatar Andrew Elble Committed by J. Bruce Fields
Browse files

nfsd: fix race with open / open upgrade stateids 



We observed multiple open stateids on the server for files that
seemingly should have been closed.

nfsd4_process_open2() tests for the existence of a preexisting
stateid. If one is not found, the locks are dropped and a new
one is created. The problem is that init_open_stateid(), which
is also responsible for hashing the newly initialized stateid,
doesn't check to see if another open has raced in and created
a matching stateid. This fix is to enable init_open_stateid() to
return the matching stateid and have nfsd4_process_open2()
swap to that stateid and switch to the open upgrade path.
In testing this patch, coverage to the newly created
path indicates that the race was indeed happening.

Signed-off-by: default avatarAndrew Elble <aweits@rit.edu>
Reviewed-by: default avatarJeff Layton <jlayton@poochiereds.net>
Signed-off-by: default avatarJ. Bruce Fields <bfields@redhat.com>
parent 34ed9872

fs/nfsd/nfs4state.c

+53 −25
Original line number Diff line number Diff line
@@ -3392,6 +3392,27 @@ static const struct nfs4_stateowner_operations openowner_ops = { 
	.so_free =	nfs4_free_openowner,

};



static struct nfs4_ol_stateid *

nfsd4_find_existing_open(struct nfs4_file *fp, struct nfsd4_open *open)

{

	struct nfs4_ol_stateid *local, *ret = NULL;

	struct nfs4_openowner *oo = open->op_openowner;



	lockdep_assert_held(&fp->fi_lock);



	list_for_each_entry(local, &fp->fi_stateids, st_perfile) {

		/* ignore lock owners */

		if (local->st_stateowner->so_is_open_owner == 0)

			continue;

		if (local->st_stateowner == &oo->oo_owner) {

			ret = local;

			atomic_inc(&ret->st_stid.sc_count);

			break;

		}

	}

	return ret;

}



static struct nfs4_openowner *

alloc_init_open_stateowner(unsigned int strhashval, struct nfsd4_open *open,

			   struct nfsd4_compound_state *cstate)
@@ -3423,9 +3444,20 @@ alloc_init_open_stateowner(unsigned int strhashval, struct nfsd4_open *open, 
	return ret;

}



static void init_open_stateid(struct nfs4_ol_stateid *stp, struct nfs4_file *fp, struct nfsd4_open *open) {

static struct nfs4_ol_stateid *

init_open_stateid(struct nfs4_ol_stateid *stp, struct nfs4_file *fp,

		struct nfsd4_open *open)

{



	struct nfs4_openowner *oo = open->op_openowner;

	struct nfs4_ol_stateid *retstp = NULL;



	spin_lock(&oo->oo_owner.so_client->cl_lock);

	spin_lock(&fp->fi_lock);



	retstp = nfsd4_find_existing_open(fp, open);

	if (retstp)

		goto out_unlock;

	atomic_inc(&stp->st_stid.sc_count);

	stp->st_stid.sc_type = NFS4_OPEN_STID;

	INIT_LIST_HEAD(&stp->st_locks);
@@ -3436,12 +3468,13 @@ static void init_open_stateid(struct nfs4_ol_stateid *stp, struct nfs4_file *fp, 
	stp->st_deny_bmap = 0;

	stp->st_openstp = NULL;

	init_rwsem(&stp->st_rwsem);

	spin_lock(&oo->oo_owner.so_client->cl_lock);

	list_add(&stp->st_perstateowner, &oo->oo_owner.so_stateids);

	spin_lock(&fp->fi_lock);

	list_add(&stp->st_perfile, &fp->fi_stateids);



out_unlock:

	spin_unlock(&fp->fi_lock);

	spin_unlock(&oo->oo_owner.so_client->cl_lock);

	return retstp;

}



/*
@@ -3852,27 +3885,6 @@ out: 
	return nfs_ok;

}



static struct nfs4_ol_stateid *

nfsd4_find_existing_open(struct nfs4_file *fp, struct nfsd4_open *open)

{

	struct nfs4_ol_stateid *local, *ret = NULL;

	struct nfs4_openowner *oo = open->op_openowner;



	spin_lock(&fp->fi_lock);

	list_for_each_entry(local, &fp->fi_stateids, st_perfile) {

		/* ignore lock owners */

		if (local->st_stateowner->so_is_open_owner == 0)

			continue;

		if (local->st_stateowner == &oo->oo_owner) {

			ret = local;

			atomic_inc(&ret->st_stid.sc_count);

			break;

		}

	}

	spin_unlock(&fp->fi_lock);

	return ret;

}



static inline int nfs4_access_to_access(u32 nfs4_access)

{

	int flags = 0;
@@ -4258,6 +4270,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf 
	struct nfs4_client *cl = open->op_openowner->oo_owner.so_client;

	struct nfs4_file *fp = NULL;

	struct nfs4_ol_stateid *stp = NULL;

	struct nfs4_ol_stateid *swapstp = NULL;

	struct nfs4_delegation *dp = NULL;

	__be32 status;
@@ -4271,7 +4284,9 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf 
		status = nfs4_check_deleg(cl, open, &dp);

		if (status)

			goto out;

		spin_lock(&fp->fi_lock);

		stp = nfsd4_find_existing_open(fp, open);

		spin_unlock(&fp->fi_lock);

	} else {

		open->op_file = NULL;

		status = nfserr_bad_stateid;
@@ -4294,7 +4309,19 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf 
	} else {

		stp = open->op_stp;

		open->op_stp = NULL;

		init_open_stateid(stp, fp, open);

		swapstp = init_open_stateid(stp, fp, open);

		if (swapstp) {

			nfs4_put_stid(&stp->st_stid);

			stp = swapstp;

			down_read(&stp->st_rwsem);

			status = nfs4_upgrade_open(rqstp, fp, current_fh,

						stp, open);

			if (status) {

				up_read(&stp->st_rwsem);

				goto out;

			}

			goto upgrade_out;

		}

		down_read(&stp->st_rwsem);

		status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open);

		if (status) {
@@ -4308,6 +4335,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf 
		if (stp->st_clnt_odstate == open->op_odstate)

			open->op_odstate = NULL;

	}

upgrade_out:

	nfs4_inc_and_copy_stateid(&open->op_stateid, &stp->st_stid);

	up_read(&stp->st_rwsem);

CRA Git | Maintained and supported by SUSTech CRA and CCSE