Commit 8dc7c311 authored by Andy Adamson's avatar Andy Adamson Committed by Trond Myklebust
Browse files

locks,lockd: fix race in nlmsvc_testlock 



posix_test_lock() returns a pointer to a struct file_lock which is unprotected
and can be removed while in use by the caller.  Move the conflicting lock from
the return to a parameter, and copy the conflicting lock.

In most cases the caller ends up putting the copy of the conflicting lock on
the stack.  On i386, sizeof(struct file_lock) appears to be about 100 bytes.
We're assuming that's reasonable.

Signed-off-by: default avatarAndy Adamson <andros@citi.umich.edu>
Signed-off-by: default avatarJ. Bruce Fields <bfields@citi.umich.edu>
Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
parent 2e0af86f

fs/lockd/svclock.c

+5 −7
Original line number Diff line number Diff line
@@ -376,8 +376,6 @@ u32 
nlmsvc_testlock(struct nlm_file *file, struct nlm_lock *lock,

				       struct nlm_lock *conflock)

{

	struct file_lock	*fl;



	dprintk("lockd: nlmsvc_testlock(%s/%ld, ty=%d, %Ld-%Ld)\n",

				file->f_file->f_dentry->d_inode->i_sb->s_id,

				file->f_file->f_dentry->d_inode->i_ino,
@@ -385,14 +383,14 @@ nlmsvc_testlock(struct nlm_file *file, struct nlm_lock *lock, 
				(long long)lock->fl.fl_start,

				(long long)lock->fl.fl_end);



	if ((fl = posix_test_lock(file->f_file, &lock->fl)) != NULL) {

	if (posix_test_lock(file->f_file, &lock->fl, &conflock->fl)) {

		dprintk("lockd: conflicting lock(ty=%d, %Ld-%Ld)\n",

				fl->fl_type, (long long)fl->fl_start,

				(long long)fl->fl_end);

				conflock->fl.fl_type,

				(long long)conflock->fl.fl_start,

				(long long)conflock->fl.fl_end);

		conflock->caller = "somehost";	/* FIXME */

		conflock->oh.len = 0;		/* don't return OH info */

		conflock->svid = fl->fl_pid;

		conflock->fl = *fl;

		conflock->svid = conflock->fl.fl_pid;

		return nlm_lck_denied;

	}

fs/locks.c

+13 −8
Original line number Diff line number Diff line
@@ -672,8 +672,9 @@ static int locks_block_on_timeout(struct file_lock *blocker, struct file_lock *w 
	return result;

}



struct file_lock *

posix_test_lock(struct file *filp, struct file_lock *fl)

int

posix_test_lock(struct file *filp, struct file_lock *fl,

		struct file_lock *conflock)

{

	struct file_lock *cfl;
@@ -684,9 +685,13 @@ posix_test_lock(struct file *filp, struct file_lock *fl) 
		if (posix_locks_conflict(cfl, fl))

			break;

	}

	if (cfl) {

		locks_copy_lock(conflock, cfl);

		unlock_kernel();



	return (cfl);

		return 1;

	}

	unlock_kernel();

	return 0;

}



EXPORT_SYMBOL(posix_test_lock);
@@ -1563,7 +1568,7 @@ asmlinkage long sys_flock(unsigned int fd, unsigned int cmd) 
 */

int fcntl_getlk(struct file *filp, struct flock __user *l)

{

	struct file_lock *fl, file_lock;

	struct file_lock *fl, cfl, file_lock;

	struct flock flock;

	int error;
@@ -1587,7 +1592,7 @@ int fcntl_getlk(struct file *filp, struct flock __user *l) 
		else

		  fl = (file_lock.fl_type == F_UNLCK ? NULL : &file_lock);

	} else {

		fl = posix_test_lock(filp, &file_lock);

		fl = (posix_test_lock(filp, &file_lock, &cfl) ? &cfl : NULL);

	}

 

	flock.l_type = F_UNLCK;
@@ -1717,7 +1722,7 @@ out: 
 */

int fcntl_getlk64(struct file *filp, struct flock64 __user *l)

{

	struct file_lock *fl, file_lock;

	struct file_lock *fl, cfl, file_lock;

	struct flock64 flock;

	int error;
@@ -1741,7 +1746,7 @@ int fcntl_getlk64(struct file *filp, struct flock64 __user *l) 
		else

		  fl = (file_lock.fl_type == F_UNLCK ? NULL : &file_lock);

	} else {

		fl = posix_test_lock(filp, &file_lock);

		fl = (posix_test_lock(filp, &file_lock, &cfl) ? &cfl : NULL);

	}

 

	flock.l_type = F_UNLCK;

fs/nfs/file.c

+3 −4
Original line number Diff line number Diff line
@@ -392,15 +392,14 @@ out_swapfile: 


static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)

{

	struct file_lock *cfl;

	struct file_lock cfl;

	struct inode *inode = filp->f_mapping->host;

	int status = 0;



	lock_kernel();

	/* Try local locking first */

	cfl = posix_test_lock(filp, fl);

	if (cfl != NULL) {

		locks_copy_lock(fl, cfl);

	if (posix_test_lock(filp, fl, &cfl)) {

		locks_copy_lock(fl, &cfl);

		goto out;

	}

fs/nfsd/nfs4state.c

+6 −7
Original line number Diff line number Diff line
@@ -2639,7 +2639,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_lock 
	struct nfs4_stateid *lock_stp;

	struct file *filp;

	struct file_lock file_lock;

	struct file_lock *conflock;

	struct file_lock conflock;

	int status = 0;

	unsigned int strhashval;
@@ -2775,11 +2775,11 @@ conflicting_lock: 
	/* XXX There is a race here. Future patch needed to provide 

	 * an atomic posix_lock_and_test_file

	 */

	if (!(conflock = posix_test_lock(filp, &file_lock))) {

	if (!posix_test_lock(filp, &file_lock, &conflock)) {

		status = nfserr_serverfault;

		goto out;

	}

	nfs4_set_lock_denied(conflock, &lock->lk_denied);

	nfs4_set_lock_denied(&conflock, &lock->lk_denied);

out:

	if (status && lock->lk_is_new && lock_sop)

		release_stateowner(lock_sop);
@@ -2800,7 +2800,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_lock 
	struct inode *inode;

	struct file file;

	struct file_lock file_lock;

	struct file_lock *conflicting_lock;

	struct file_lock conflock;

	int status;



	if (nfs4_in_grace())
@@ -2864,10 +2864,9 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_lock 
	file.f_dentry = current_fh->fh_dentry;



	status = nfs_ok;

	conflicting_lock = posix_test_lock(&file, &file_lock);

	if (conflicting_lock) {

	if (posix_test_lock(&file, &file_lock, &conflock)) {

		status = nfserr_denied;

		nfs4_set_lock_denied(conflicting_lock, &lockt->lt_denied);

		nfs4_set_lock_denied(&conflock, &lockt->lt_denied);

	}

out:

	nfs4_unlock_state();

include/linux/fs.h

+1 −1
Original line number Diff line number Diff line
@@ -754,7 +754,7 @@ extern void locks_init_lock(struct file_lock *); 
extern void locks_copy_lock(struct file_lock *, struct file_lock *);

extern void locks_remove_posix(struct file *, fl_owner_t);

extern void locks_remove_flock(struct file *);

extern struct file_lock *posix_test_lock(struct file *, struct file_lock *);

extern int posix_test_lock(struct file *, struct file_lock *, struct file_lock *);

extern int posix_lock_file(struct file *, struct file_lock *);

extern int posix_lock_file_wait(struct file *, struct file_lock *);

extern int posix_unblock_lock(struct file *, struct file_lock *);

CRA Git | Maintained and supported by SUSTech CRA and CCSE