2
0
mirror of https://github.com/edk2-porting/linux-next.git synced 2024-12-30 16:13:54 +08:00

nfsd: make deny mode enforcement more efficient and close races in it

The current enforcement of deny modes is both inefficient and scattered
across several places, which makes it hard to guarantee atomicity. The
inefficiency is a problem now, and the lack of atomicity will mean races
once the client_mutex is removed.

First, we address the inefficiency. We have to track deny modes on a
per-stateid basis to ensure that open downgrades are sane, but when the
server goes to enforce them it has to walk the entire list of stateids
and check against each one.

Instead of doing that, maintain a per-nfs4_file deny mode. When a file
is opened, we simply set any deny bits in that mode that were specified
in the OPEN call. We can then use that unified deny mode to do a simple
check to see whether there are any conflicts without needing to walk the
entire stateid list.

The only time we'll need to walk the entire list of stateids is when a
stateid that has a deny mode on it is being released, or one is having
its deny mode downgraded. In that case, we must walk the entire list and
recalculate the fi_share_deny field. Since deny modes are pretty rare
today, this should be very rare under normal workloads.

To address the potential for races once the client_mutex is removed,
protect fi_share_deny with the fi_lock. In nfs4_get_vfs_file, check to
make sure that any deny mode we want to apply won't conflict with
existing access. If that's ok, then have nfs4_file_get_access check that
new access to the file won't conflict with existing deny modes.

If that also passes, then get file access references, set the correct
access and deny bits in the stateid, and update the fi_share_deny field.
If opening the file or truncating it fails, then unwind the whole mess
and return the appropriate error.

Signed-off-by: Jeff Layton <jlayton@primarydata.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
This commit is contained in:
Jeff Layton 2014-07-10 14:07:34 -04:00 committed by J. Bruce Fields
parent 7214e8600e
commit baeb4ff0e5
2 changed files with 125 additions and 58 deletions

View File

@ -394,10 +394,33 @@ nfs4_file_get_access(struct nfs4_file *fp, u32 access)
if (access & ~NFS4_SHARE_ACCESS_BOTH)
return nfserr_inval;
/* Does it conflict with a deny mode already set? */
if ((access & fp->fi_share_deny) != 0)
return nfserr_share_denied;
__nfs4_file_get_access(fp, access);
return nfs_ok;
}
static __be32 nfs4_file_check_deny(struct nfs4_file *fp, u32 deny)
{
/* Common case is that there is no deny mode. */
if (deny) {
/* Does this deny mode make sense? */
if (deny & ~NFS4_SHARE_DENY_BOTH)
return nfserr_inval;
if ((deny & NFS4_SHARE_DENY_READ) &&
atomic_read(&fp->fi_access[O_RDONLY]))
return nfserr_share_denied;
if ((deny & NFS4_SHARE_DENY_WRITE) &&
atomic_read(&fp->fi_access[O_WRONLY]))
return nfserr_share_denied;
}
return nfs_ok;
}
static void __nfs4_file_put_access(struct nfs4_file *fp, int oflag)
{
might_lock(&fp->fi_lock);
@ -710,17 +733,6 @@ bmap_to_share_mode(unsigned long bmap) {
return access;
}
static bool
test_share(struct nfs4_ol_stateid *stp, struct nfsd4_open *open) {
unsigned int access, deny;
access = bmap_to_share_mode(stp->st_access_bmap);
deny = bmap_to_share_mode(stp->st_deny_bmap);
if ((access & open->op_share_deny) || (deny & open->op_share_access))
return false;
return true;
}
/* set share access for a given stateid */
static inline void
set_access(u32 access, struct nfs4_ol_stateid *stp)
@ -793,11 +805,49 @@ static int nfs4_access_to_omode(u32 access)
return O_RDONLY;
}
/*
* A stateid that had a deny mode associated with it is being released
* or downgraded. Recalculate the deny mode on the file.
*/
static void
recalculate_deny_mode(struct nfs4_file *fp)
{
struct nfs4_ol_stateid *stp;
spin_lock(&fp->fi_lock);
fp->fi_share_deny = 0;
list_for_each_entry(stp, &fp->fi_stateids, st_perfile)
fp->fi_share_deny |= bmap_to_share_mode(stp->st_deny_bmap);
spin_unlock(&fp->fi_lock);
}
static void
reset_union_bmap_deny(u32 deny, struct nfs4_ol_stateid *stp)
{
int i;
bool change = false;
for (i = 1; i < 4; i++) {
if ((i & deny) != i) {
change = true;
clear_deny(i, stp);
}
}
/* Recalculate per-file deny mode if there was a change */
if (change)
recalculate_deny_mode(stp->st_file);
}
/* release all access and file references for a given stateid */
static void
release_all_access(struct nfs4_ol_stateid *stp)
{
int i;
struct nfs4_file *fp = stp->st_file;
if (fp && stp->st_deny_bmap != 0)
recalculate_deny_mode(fp);
for (i = 1; i < 4; i++) {
if (test_access(i, stp))
@ -2787,6 +2837,7 @@ static void nfsd4_init_file(struct nfs4_file *fp, struct inode *ino)
fp->fi_inode = ino;
fp->fi_had_conflict = false;
fp->fi_lease = NULL;
fp->fi_share_deny = 0;
memset(fp->fi_fds, 0, sizeof(fp->fi_fds));
memset(fp->fi_access, 0, sizeof(fp->fi_access));
hlist_add_head(&fp->fi_hash, &file_hashtbl[hashval]);
@ -3014,22 +3065,15 @@ nfs4_share_conflict(struct svc_fh *current_fh, unsigned int deny_type)
{
struct inode *ino = current_fh->fh_dentry->d_inode;
struct nfs4_file *fp;
struct nfs4_ol_stateid *stp;
__be32 ret;
__be32 ret = nfs_ok;
fp = find_file(ino);
if (!fp)
return nfs_ok;
ret = nfserr_locked;
/* Search for conflicting share reservations */
return ret;
/* Check for conflicting share reservations */
spin_lock(&fp->fi_lock);
list_for_each_entry(stp, &fp->fi_stateids, st_perfile) {
if (test_deny(deny_type, stp) ||
test_deny(NFS4_SHARE_DENY_BOTH, stp))
goto out;
}
ret = nfs_ok;
out:
if (fp->fi_share_deny & deny_type)
ret = nfserr_locked;
spin_unlock(&fp->fi_lock);
put_nfs4_file(fp);
return ret;
@ -3265,12 +3309,9 @@ nfs4_check_open(struct nfs4_file *fp, struct nfsd4_open *open, struct nfs4_ol_st
if (local->st_stateowner->so_is_open_owner == 0)
continue;
/* remember if we have seen this open owner */
if (local->st_stateowner == &oo->oo_owner)
if (local->st_stateowner == &oo->oo_owner) {
*stpp = local;
/* check for conflicting share reservations */
if (!test_share(local, open)) {
spin_unlock(&fp->fi_lock);
return nfserr_share_denied;
break;
}
}
spin_unlock(&fp->fi_lock);
@ -3311,56 +3352,91 @@ static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,
__be32 status;
int oflag = nfs4_access_to_omode(open->op_share_access);
int access = nfs4_access_to_access(open->op_share_access);
unsigned char old_access_bmap, old_deny_bmap;
spin_lock(&fp->fi_lock);
/*
* Are we trying to set a deny mode that would conflict with
* current access?
*/
status = nfs4_file_check_deny(fp, open->op_share_deny);
if (status != nfs_ok) {
spin_unlock(&fp->fi_lock);
goto out;
}
/* set access to the file */
status = nfs4_file_get_access(fp, open->op_share_access);
if (status != nfs_ok) {
spin_unlock(&fp->fi_lock);
goto out;
}
/* Set access bits in stateid */
old_access_bmap = stp->st_access_bmap;
set_access(open->op_share_access, stp);
/* Set new deny mask */
old_deny_bmap = stp->st_deny_bmap;
set_deny(open->op_share_deny, stp);
fp->fi_share_deny |= (open->op_share_deny & NFS4_SHARE_DENY_BOTH);
if (!fp->fi_fds[oflag]) {
spin_unlock(&fp->fi_lock);
status = nfsd_open(rqstp, cur_fh, S_IFREG, access, &filp);
if (status)
goto out;
goto out_put_access;
spin_lock(&fp->fi_lock);
if (!fp->fi_fds[oflag]) {
fp->fi_fds[oflag] = filp;
filp = NULL;
}
}
status = nfs4_file_get_access(fp, open->op_share_access);
spin_unlock(&fp->fi_lock);
if (filp)
fput(filp);
if (status)
goto out_put_access;
status = nfsd4_truncate(rqstp, cur_fh, open);
if (status)
goto out_put_access;
/* Set access and deny bits in stateid */
set_access(open->op_share_access, stp);
set_deny(open->op_share_deny, stp);
return nfs_ok;
out_put_access:
nfs4_file_put_access(fp, open->op_share_access);
out:
return status;
out_put_access:
stp->st_access_bmap = old_access_bmap;
nfs4_file_put_access(fp, open->op_share_access);
reset_union_bmap_deny(bmap_to_share_mode(old_deny_bmap), stp);
goto out;
}
static __be32
nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *cur_fh, struct nfs4_ol_stateid *stp, struct nfsd4_open *open)
{
__be32 status;
unsigned char old_deny_bmap;
if (!test_access(open->op_share_access, stp))
status = nfs4_get_vfs_file(rqstp, fp, cur_fh, stp, open);
else
status = nfsd4_truncate(rqstp, cur_fh, open);
return nfs4_get_vfs_file(rqstp, fp, cur_fh, stp, open);
if (status)
/* test and set deny mode */
spin_lock(&fp->fi_lock);
status = nfs4_file_check_deny(fp, open->op_share_deny);
if (status == nfs_ok) {
old_deny_bmap = stp->st_deny_bmap;
set_deny(open->op_share_deny, stp);
fp->fi_share_deny |=
(open->op_share_deny & NFS4_SHARE_DENY_BOTH);
}
spin_unlock(&fp->fi_lock);
if (status != nfs_ok)
return status;
return nfs_ok;
}
status = nfsd4_truncate(rqstp, cur_fh, open);
if (status != nfs_ok)
reset_union_bmap_deny(old_deny_bmap, stp);
return status;
}
static void
nfs4_set_claim_prev(struct nfsd4_open *open, bool has_session)
@ -3582,7 +3658,8 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
*/
fp = find_or_add_file(ino, open->op_file);
if (fp != open->op_file) {
if ((status = nfs4_check_open(fp, open, &stp)))
status = nfs4_check_open(fp, open, &stp);
if (status)
goto out;
status = nfs4_check_deleg(cl, open, &dp);
if (status)
@ -4269,17 +4346,6 @@ static inline void nfs4_stateid_downgrade(struct nfs4_ol_stateid *stp, u32 to_ac
}
}
static void
reset_union_bmap_deny(u32 deny, struct nfs4_ol_stateid *stp)
{
int i;
for (i = 1; i < 4; i++) {
if ((i & deny) != i)
clear_deny(i, stp);
}
}
__be32
nfsd4_open_downgrade(struct svc_rqst *rqstp,
struct nfsd4_compound_state *cstate,

View File

@ -391,6 +391,7 @@ struct nfs4_file {
* + 1 to both of the above if NFS4_SHARE_ACCESS_BOTH is set.
*/
atomic_t fi_access[2];
u32 fi_share_deny;
struct file *fi_deleg_file;
struct file_lock *fi_lease;
atomic_t fi_delegees;