linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] locks: ensure that fl_owner is always initialized properly in flock and lease codepaths
@ 2014-04-28 17:50 Jeff Layton
  2014-04-28 18:15 ` Greg Kroah-Hartman
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jeff Layton @ 2014-04-28 17:50 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Greg Kroah-Hartman, Eric Van Hensbergen, Ron Minnich,
	Latchesar Ionkov, David Howells, Sage Weil, Miklos Szeredi,
	J. Bruce Fields, Alexander Viro, Trond Myklebust,
	open list:STAGING SUBSYSTEM, open list, open list:9P FILE SYSTEM,
	open list:AFS FILESYSTEM &..., open list:CEPH DISTRIBUTED...,
	open list:FUSE: FILESYSTEM..., open list:NFS, SUNRPC, AND...

Currently, the fl_owner isn't set for flock locks. Some filesystems use
byte-range locks to simulate flock locks and there is a common idiom in
those that does:

    fl->fl_owner = (fl_owner_t)filp;
    fl->fl_start = 0;
    fl->fl_end = OFFSET_MAX;

Since flock locks are generally "owned" by the open file description,
move this into the common flock lock setup code. The fl_start and fl_end
fields are already set appropriately, so remove the unneeded setting of
that in flock ops in those filesystems as well.

Finally, the lease code also sets the fl_owner as if they were owned by
the process and not the open file description. This is incorrect as
leases have the same ownership semantics as flock locks. Set them the
same way. The lease code doesn't actually use the fl_owner value for
anything, so this is more for consistency's sake than a bugfix.

Reported-by: Trond Myklebust <trond.myklebust@primarydata.com>
Signed-off-by: Jeff Layton <jlayton@poochiereds.net>
---
 drivers/staging/lustre/lustre/llite/file.c | 17 ++++++-----------
 fs/9p/vfs_file.c                           |  3 ---
 fs/afs/flock.c                             |  4 ----
 fs/ceph/locks.c                            | 10 ++--------
 fs/fuse/file.c                             |  1 -
 fs/locks.c                                 |  4 +++-
 fs/nfs/file.c                              |  4 ----
 7 files changed, 11 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
index 8e844a6371e0..760ccd83f1f7 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -2691,20 +2691,15 @@ int ll_file_flock(struct file *file, int cmd, struct file_lock *file_lock)
 
 	ll_stats_ops_tally(ll_i2sbi(inode), LPROC_LL_FLOCK, 1);
 
-	if (file_lock->fl_flags & FL_FLOCK) {
+	if (file_lock->fl_flags & FL_FLOCK)
 		LASSERT((cmd == F_SETLKW) || (cmd == F_SETLK));
-		/* flocks are whole-file locks */
-		flock.l_flock.end = OFFSET_MAX;
-		/* For flocks owner is determined by the local file descriptor*/
-		flock.l_flock.owner = (unsigned long)file_lock->fl_file;
-	} else if (file_lock->fl_flags & FL_POSIX) {
-		flock.l_flock.owner = (unsigned long)file_lock->fl_owner;
-		flock.l_flock.start = file_lock->fl_start;
-		flock.l_flock.end = file_lock->fl_end;
-	} else {
+	else if (!(file_lock->fl_flags & FL_POSIX))
 		return -EINVAL;
-	}
+
+	flock.l_flock.owner = (unsigned long)file_lock->fl_owner;
 	flock.l_flock.pid = file_lock->fl_pid;
+	flock.l_flock.start = file_lock->fl_start;
+	flock.l_flock.end = file_lock->fl_end;
 
 	/* Somewhat ugly workaround for svc lockd.
 	 * lockd installs custom fl_lmops->lm_compare_owner that checks
diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
index d8223209d4b1..59e3fe3d56c0 100644
--- a/fs/9p/vfs_file.c
+++ b/fs/9p/vfs_file.c
@@ -352,9 +352,6 @@ static int v9fs_file_flock_dotl(struct file *filp, int cmd,
 		invalidate_mapping_pages(&inode->i_data, 0, -1);
 	}
 	/* Convert flock to posix lock */
-	fl->fl_owner = (fl_owner_t)filp;
-	fl->fl_start = 0;
-	fl->fl_end = OFFSET_MAX;
 	fl->fl_flags |= FL_POSIX;
 	fl->fl_flags ^= FL_FLOCK;
 
diff --git a/fs/afs/flock.c b/fs/afs/flock.c
index a8cf2cff836c..4baf1d2b39e4 100644
--- a/fs/afs/flock.c
+++ b/fs/afs/flock.c
@@ -555,10 +555,6 @@ int afs_flock(struct file *file, int cmd, struct file_lock *fl)
 		return -ENOLCK;
 
 	/* we're simulating flock() locks using posix locks on the server */
-	fl->fl_owner = (fl_owner_t) file;
-	fl->fl_start = 0;
-	fl->fl_end = OFFSET_MAX;
-
 	if (fl->fl_type == F_UNLCK)
 		return afs_do_unlk(file, fl);
 	return afs_do_setlk(file, fl);
diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c
index d94ba0df9f4d..db8c1ca15d72 100644
--- a/fs/ceph/locks.c
+++ b/fs/ceph/locks.c
@@ -52,10 +52,7 @@ static int ceph_lock_message(u8 lock_type, u16 operation, struct file *file,
 	else
 		length = fl->fl_end - fl->fl_start + 1;
 
-	if (lock_type == CEPH_LOCK_FCNTL)
-		owner = secure_addr(fl->fl_owner);
-	else
-		owner = secure_addr(fl->fl_file);
+	owner = secure_addr(fl->fl_owner);
 
 	dout("ceph_lock_message: rule: %d, op: %d, owner: %llx, pid: %llu, "
 	     "start: %llu, length: %llu, wait: %d, type: %d", (int)lock_type,
@@ -313,10 +310,7 @@ int lock_to_ceph_filelock(struct file_lock *lock,
 	cephlock->length = cpu_to_le64(lock->fl_end - lock->fl_start + 1);
 	cephlock->client = cpu_to_le64(0);
 	cephlock->pid = cpu_to_le64((u64)lock->fl_pid);
-	if (lock->fl_flags & FL_POSIX)
-		cephlock->owner = cpu_to_le64(secure_addr(lock->fl_owner));
-	else
-		cephlock->owner = cpu_to_le64(secure_addr(lock->fl_file));
+	cephlock->owner = cpu_to_le64(secure_addr(lock->fl_owner));
 
 	switch (lock->fl_type) {
 	case F_RDLCK:
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 13f8bdec5110..0b25fec89558 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2281,7 +2281,6 @@ static int fuse_file_flock(struct file *file, int cmd, struct file_lock *fl)
 		struct fuse_file *ff = file->private_data;
 
 		/* emulate flock with POSIX locks */
-		fl->fl_owner = (fl_owner_t) file;
 		ff->flock = true;
 		err = fuse_setlk(file, fl, 1);
 	}
diff --git a/fs/locks.c b/fs/locks.c
index e663aeac579e..2129ba8ac062 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -322,6 +322,7 @@ static int flock_make_lock(struct file *filp, struct file_lock **lock,
 		return -ENOMEM;
 
 	fl->fl_file = filp;
+	fl->fl_owner = (fl_owner_t)filp;
 	fl->fl_pid = current->tgid;
 	fl->fl_flags = FL_FLOCK;
 	fl->fl_type = type;
@@ -439,7 +440,7 @@ static int lease_init(struct file *filp, long type, struct file_lock *fl)
 	if (assign_type(fl, type) != 0)
 		return -EINVAL;
 
-	fl->fl_owner = current->files;
+	fl->fl_owner = (fl_owner_t)filp;
 	fl->fl_pid = current->tgid;
 
 	fl->fl_file = filp;
@@ -2304,6 +2305,7 @@ void locks_remove_file(struct file *filp)
 
 	if (filp->f_op->flock) {
 		struct file_lock fl = {
+			.fl_owner = (fl_owner_t)filp,
 			.fl_pid = current->tgid,
 			.fl_file = filp,
 			.fl_flags = FL_FLOCK,
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 284ca901fe16..c1edf7336315 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -916,10 +916,6 @@ int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 		is_local = 1;
 
 	/* We're simulating flock() locks using posix locks on the server */
-	fl->fl_owner = (fl_owner_t)filp;
-	fl->fl_start = 0;
-	fl->fl_end = OFFSET_MAX;
-
 	if (fl->fl_type == F_UNLCK)
 		return do_unlk(filp, cmd, fl, is_local);
 	return do_setlk(filp, cmd, fl, is_local);
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] locks: ensure that fl_owner is always initialized properly in flock and lease codepaths
  2014-04-28 17:50 [PATCH] locks: ensure that fl_owner is always initialized properly in flock and lease codepaths Jeff Layton
@ 2014-04-28 18:15 ` Greg Kroah-Hartman
  2014-05-05 14:29 ` J. Bruce Fields
  2014-05-06 21:45 ` Gregory Farnum
  2 siblings, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2014-04-28 18:15 UTC (permalink / raw)
  To: Jeff Layton
  Cc: linux-fsdevel, Eric Van Hensbergen, Ron Minnich,
	Latchesar Ionkov, David Howells, Sage Weil, Miklos Szeredi,
	J. Bruce Fields, Alexander Viro, Trond Myklebust,
	open list:STAGING SUBSYSTEM, open list, open list:9P FILE SYSTEM,
	open list:AFS FILESYSTEM &..., open list:CEPH DISTRIBUTED...,
	open list:FUSE: FILESYSTEM..., open list:NFS, SUNRPC, AND...

On Mon, Apr 28, 2014 at 01:50:13PM -0400, Jeff Layton wrote:
> Currently, the fl_owner isn't set for flock locks. Some filesystems use
> byte-range locks to simulate flock locks and there is a common idiom in
> those that does:
> 
>     fl->fl_owner = (fl_owner_t)filp;
>     fl->fl_start = 0;
>     fl->fl_end = OFFSET_MAX;
> 
> Since flock locks are generally "owned" by the open file description,
> move this into the common flock lock setup code. The fl_start and fl_end
> fields are already set appropriately, so remove the unneeded setting of
> that in flock ops in those filesystems as well.
> 
> Finally, the lease code also sets the fl_owner as if they were owned by
> the process and not the open file description. This is incorrect as
> leases have the same ownership semantics as flock locks. Set them the
> same way. The lease code doesn't actually use the fl_owner value for
> anything, so this is more for consistency's sake than a bugfix.
> 
> Reported-by: Trond Myklebust <trond.myklebust@primarydata.com>
> Signed-off-by: Jeff Layton <jlayton@poochiereds.net>
> ---
>  drivers/staging/lustre/lustre/llite/file.c | 17 ++++++-----------
>  fs/9p/vfs_file.c                           |  3 ---
>  fs/afs/flock.c                             |  4 ----
>  fs/ceph/locks.c                            | 10 ++--------
>  fs/fuse/file.c                             |  1 -
>  fs/locks.c                                 |  4 +++-
>  fs/nfs/file.c                              |  4 ----
>  7 files changed, 11 insertions(+), 32 deletions(-)

Staging portion:

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] locks: ensure that fl_owner is always initialized properly in flock and lease codepaths
  2014-04-28 17:50 [PATCH] locks: ensure that fl_owner is always initialized properly in flock and lease codepaths Jeff Layton
  2014-04-28 18:15 ` Greg Kroah-Hartman
@ 2014-05-05 14:29 ` J. Bruce Fields
  2014-05-06 21:45 ` Gregory Farnum
  2 siblings, 0 replies; 4+ messages in thread
From: J. Bruce Fields @ 2014-05-05 14:29 UTC (permalink / raw)
  To: Jeff Layton
  Cc: linux-fsdevel, Greg Kroah-Hartman, Eric Van Hensbergen,
	Ron Minnich, Latchesar Ionkov, David Howells, Sage Weil,
	Miklos Szeredi, Alexander Viro, Trond Myklebust,
	open list:STAGING SUBSYSTEM, open list, open list:9P FILE SYSTEM,
	open list:AFS FILESYSTEM &..., open list:CEPH DISTRIBUTED...,
	open list:FUSE: FILESYSTEM..., open list:NFS, SUNRPC, AND...

Looks fine to me, ACK.--b.

On Mon, Apr 28, 2014 at 01:50:13PM -0400, Jeff Layton wrote:
> Currently, the fl_owner isn't set for flock locks. Some filesystems use
> byte-range locks to simulate flock locks and there is a common idiom in
> those that does:
> 
>     fl->fl_owner = (fl_owner_t)filp;
>     fl->fl_start = 0;
>     fl->fl_end = OFFSET_MAX;
> 
> Since flock locks are generally "owned" by the open file description,
> move this into the common flock lock setup code. The fl_start and fl_end
> fields are already set appropriately, so remove the unneeded setting of
> that in flock ops in those filesystems as well.
> 
> Finally, the lease code also sets the fl_owner as if they were owned by
> the process and not the open file description. This is incorrect as
> leases have the same ownership semantics as flock locks. Set them the
> same way. The lease code doesn't actually use the fl_owner value for
> anything, so this is more for consistency's sake than a bugfix.
> 
> Reported-by: Trond Myklebust <trond.myklebust@primarydata.com>
> Signed-off-by: Jeff Layton <jlayton@poochiereds.net>
> ---
>  drivers/staging/lustre/lustre/llite/file.c | 17 ++++++-----------
>  fs/9p/vfs_file.c                           |  3 ---
>  fs/afs/flock.c                             |  4 ----
>  fs/ceph/locks.c                            | 10 ++--------
>  fs/fuse/file.c                             |  1 -
>  fs/locks.c                                 |  4 +++-
>  fs/nfs/file.c                              |  4 ----
>  7 files changed, 11 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
> index 8e844a6371e0..760ccd83f1f7 100644
> --- a/drivers/staging/lustre/lustre/llite/file.c
> +++ b/drivers/staging/lustre/lustre/llite/file.c
> @@ -2691,20 +2691,15 @@ int ll_file_flock(struct file *file, int cmd, struct file_lock *file_lock)
>  
>  	ll_stats_ops_tally(ll_i2sbi(inode), LPROC_LL_FLOCK, 1);
>  
> -	if (file_lock->fl_flags & FL_FLOCK) {
> +	if (file_lock->fl_flags & FL_FLOCK)
>  		LASSERT((cmd == F_SETLKW) || (cmd == F_SETLK));
> -		/* flocks are whole-file locks */
> -		flock.l_flock.end = OFFSET_MAX;
> -		/* For flocks owner is determined by the local file descriptor*/
> -		flock.l_flock.owner = (unsigned long)file_lock->fl_file;
> -	} else if (file_lock->fl_flags & FL_POSIX) {
> -		flock.l_flock.owner = (unsigned long)file_lock->fl_owner;
> -		flock.l_flock.start = file_lock->fl_start;
> -		flock.l_flock.end = file_lock->fl_end;
> -	} else {
> +	else if (!(file_lock->fl_flags & FL_POSIX))
>  		return -EINVAL;
> -	}
> +
> +	flock.l_flock.owner = (unsigned long)file_lock->fl_owner;
>  	flock.l_flock.pid = file_lock->fl_pid;
> +	flock.l_flock.start = file_lock->fl_start;
> +	flock.l_flock.end = file_lock->fl_end;
>  
>  	/* Somewhat ugly workaround for svc lockd.
>  	 * lockd installs custom fl_lmops->lm_compare_owner that checks
> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
> index d8223209d4b1..59e3fe3d56c0 100644
> --- a/fs/9p/vfs_file.c
> +++ b/fs/9p/vfs_file.c
> @@ -352,9 +352,6 @@ static int v9fs_file_flock_dotl(struct file *filp, int cmd,
>  		invalidate_mapping_pages(&inode->i_data, 0, -1);
>  	}
>  	/* Convert flock to posix lock */
> -	fl->fl_owner = (fl_owner_t)filp;
> -	fl->fl_start = 0;
> -	fl->fl_end = OFFSET_MAX;
>  	fl->fl_flags |= FL_POSIX;
>  	fl->fl_flags ^= FL_FLOCK;
>  
> diff --git a/fs/afs/flock.c b/fs/afs/flock.c
> index a8cf2cff836c..4baf1d2b39e4 100644
> --- a/fs/afs/flock.c
> +++ b/fs/afs/flock.c
> @@ -555,10 +555,6 @@ int afs_flock(struct file *file, int cmd, struct file_lock *fl)
>  		return -ENOLCK;
>  
>  	/* we're simulating flock() locks using posix locks on the server */
> -	fl->fl_owner = (fl_owner_t) file;
> -	fl->fl_start = 0;
> -	fl->fl_end = OFFSET_MAX;
> -
>  	if (fl->fl_type == F_UNLCK)
>  		return afs_do_unlk(file, fl);
>  	return afs_do_setlk(file, fl);
> diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c
> index d94ba0df9f4d..db8c1ca15d72 100644
> --- a/fs/ceph/locks.c
> +++ b/fs/ceph/locks.c
> @@ -52,10 +52,7 @@ static int ceph_lock_message(u8 lock_type, u16 operation, struct file *file,
>  	else
>  		length = fl->fl_end - fl->fl_start + 1;
>  
> -	if (lock_type == CEPH_LOCK_FCNTL)
> -		owner = secure_addr(fl->fl_owner);
> -	else
> -		owner = secure_addr(fl->fl_file);
> +	owner = secure_addr(fl->fl_owner);
>  
>  	dout("ceph_lock_message: rule: %d, op: %d, owner: %llx, pid: %llu, "
>  	     "start: %llu, length: %llu, wait: %d, type: %d", (int)lock_type,
> @@ -313,10 +310,7 @@ int lock_to_ceph_filelock(struct file_lock *lock,
>  	cephlock->length = cpu_to_le64(lock->fl_end - lock->fl_start + 1);
>  	cephlock->client = cpu_to_le64(0);
>  	cephlock->pid = cpu_to_le64((u64)lock->fl_pid);
> -	if (lock->fl_flags & FL_POSIX)
> -		cephlock->owner = cpu_to_le64(secure_addr(lock->fl_owner));
> -	else
> -		cephlock->owner = cpu_to_le64(secure_addr(lock->fl_file));
> +	cephlock->owner = cpu_to_le64(secure_addr(lock->fl_owner));
>  
>  	switch (lock->fl_type) {
>  	case F_RDLCK:
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 13f8bdec5110..0b25fec89558 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -2281,7 +2281,6 @@ static int fuse_file_flock(struct file *file, int cmd, struct file_lock *fl)
>  		struct fuse_file *ff = file->private_data;
>  
>  		/* emulate flock with POSIX locks */
> -		fl->fl_owner = (fl_owner_t) file;
>  		ff->flock = true;
>  		err = fuse_setlk(file, fl, 1);
>  	}
> diff --git a/fs/locks.c b/fs/locks.c
> index e663aeac579e..2129ba8ac062 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -322,6 +322,7 @@ static int flock_make_lock(struct file *filp, struct file_lock **lock,
>  		return -ENOMEM;
>  
>  	fl->fl_file = filp;
> +	fl->fl_owner = (fl_owner_t)filp;
>  	fl->fl_pid = current->tgid;
>  	fl->fl_flags = FL_FLOCK;
>  	fl->fl_type = type;
> @@ -439,7 +440,7 @@ static int lease_init(struct file *filp, long type, struct file_lock *fl)
>  	if (assign_type(fl, type) != 0)
>  		return -EINVAL;
>  
> -	fl->fl_owner = current->files;
> +	fl->fl_owner = (fl_owner_t)filp;
>  	fl->fl_pid = current->tgid;
>  
>  	fl->fl_file = filp;
> @@ -2304,6 +2305,7 @@ void locks_remove_file(struct file *filp)
>  
>  	if (filp->f_op->flock) {
>  		struct file_lock fl = {
> +			.fl_owner = (fl_owner_t)filp,
>  			.fl_pid = current->tgid,
>  			.fl_file = filp,
>  			.fl_flags = FL_FLOCK,
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 284ca901fe16..c1edf7336315 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -916,10 +916,6 @@ int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>  		is_local = 1;
>  
>  	/* We're simulating flock() locks using posix locks on the server */
> -	fl->fl_owner = (fl_owner_t)filp;
> -	fl->fl_start = 0;
> -	fl->fl_end = OFFSET_MAX;
> -
>  	if (fl->fl_type == F_UNLCK)
>  		return do_unlk(filp, cmd, fl, is_local);
>  	return do_setlk(filp, cmd, fl, is_local);
> -- 
> 1.9.0
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] locks: ensure that fl_owner is always initialized properly in flock and lease codepaths
  2014-04-28 17:50 [PATCH] locks: ensure that fl_owner is always initialized properly in flock and lease codepaths Jeff Layton
  2014-04-28 18:15 ` Greg Kroah-Hartman
  2014-05-05 14:29 ` J. Bruce Fields
@ 2014-05-06 21:45 ` Gregory Farnum
  2 siblings, 0 replies; 4+ messages in thread
From: Gregory Farnum @ 2014-05-06 21:45 UTC (permalink / raw)
  To: Jeff Layton
  Cc: linux-fsdevel, Greg Kroah-Hartman, Eric Van Hensbergen,
	Ron Minnich, Latchesar Ionkov, David Howells, Sage Weil,
	Miklos Szeredi, J. Bruce Fields, Alexander Viro, Trond Myklebust,
	open list:STAGING SUBSYSTEM, open list, open list:9P FILE SYSTEM,
	open list:AFS FILESYSTEM &..., open list:CEPH DISTRIBUTED...,
	open list:FUSE: FILESYSTEM..., open list:NFS, SUNRPC, AND...

The Ceph bit is fine.
Acked-by: Greg Farnum <greg@inktank.com>


On Mon, Apr 28, 2014 at 10:50 AM, Jeff Layton <jlayton@poochiereds.net> wrote:
> Currently, the fl_owner isn't set for flock locks. Some filesystems use
> byte-range locks to simulate flock locks and there is a common idiom in
> those that does:
>
>     fl->fl_owner = (fl_owner_t)filp;
>     fl->fl_start = 0;
>     fl->fl_end = OFFSET_MAX;
>
> Since flock locks are generally "owned" by the open file description,
> move this into the common flock lock setup code. The fl_start and fl_end
> fields are already set appropriately, so remove the unneeded setting of
> that in flock ops in those filesystems as well.
>
> Finally, the lease code also sets the fl_owner as if they were owned by
> the process and not the open file description. This is incorrect as
> leases have the same ownership semantics as flock locks. Set them the
> same way. The lease code doesn't actually use the fl_owner value for
> anything, so this is more for consistency's sake than a bugfix.
>
> Reported-by: Trond Myklebust <trond.myklebust@primarydata.com>
> Signed-off-by: Jeff Layton <jlayton@poochiereds.net>
> ---
>  drivers/staging/lustre/lustre/llite/file.c | 17 ++++++-----------
>  fs/9p/vfs_file.c                           |  3 ---
>  fs/afs/flock.c                             |  4 ----
>  fs/ceph/locks.c                            | 10 ++--------
>  fs/fuse/file.c                             |  1 -
>  fs/locks.c                                 |  4 +++-
>  fs/nfs/file.c                              |  4 ----
>  7 files changed, 11 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
> index 8e844a6371e0..760ccd83f1f7 100644
> --- a/drivers/staging/lustre/lustre/llite/file.c
> +++ b/drivers/staging/lustre/lustre/llite/file.c
> @@ -2691,20 +2691,15 @@ int ll_file_flock(struct file *file, int cmd, struct file_lock *file_lock)
>
>         ll_stats_ops_tally(ll_i2sbi(inode), LPROC_LL_FLOCK, 1);
>
> -       if (file_lock->fl_flags & FL_FLOCK) {
> +       if (file_lock->fl_flags & FL_FLOCK)
>                 LASSERT((cmd == F_SETLKW) || (cmd == F_SETLK));
> -               /* flocks are whole-file locks */
> -               flock.l_flock.end = OFFSET_MAX;
> -               /* For flocks owner is determined by the local file descriptor*/
> -               flock.l_flock.owner = (unsigned long)file_lock->fl_file;
> -       } else if (file_lock->fl_flags & FL_POSIX) {
> -               flock.l_flock.owner = (unsigned long)file_lock->fl_owner;
> -               flock.l_flock.start = file_lock->fl_start;
> -               flock.l_flock.end = file_lock->fl_end;
> -       } else {
> +       else if (!(file_lock->fl_flags & FL_POSIX))
>                 return -EINVAL;
> -       }
> +
> +       flock.l_flock.owner = (unsigned long)file_lock->fl_owner;
>         flock.l_flock.pid = file_lock->fl_pid;
> +       flock.l_flock.start = file_lock->fl_start;
> +       flock.l_flock.end = file_lock->fl_end;
>
>         /* Somewhat ugly workaround for svc lockd.
>          * lockd installs custom fl_lmops->lm_compare_owner that checks
> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
> index d8223209d4b1..59e3fe3d56c0 100644
> --- a/fs/9p/vfs_file.c
> +++ b/fs/9p/vfs_file.c
> @@ -352,9 +352,6 @@ static int v9fs_file_flock_dotl(struct file *filp, int cmd,
>                 invalidate_mapping_pages(&inode->i_data, 0, -1);
>         }
>         /* Convert flock to posix lock */
> -       fl->fl_owner = (fl_owner_t)filp;
> -       fl->fl_start = 0;
> -       fl->fl_end = OFFSET_MAX;
>         fl->fl_flags |= FL_POSIX;
>         fl->fl_flags ^= FL_FLOCK;
>
> diff --git a/fs/afs/flock.c b/fs/afs/flock.c
> index a8cf2cff836c..4baf1d2b39e4 100644
> --- a/fs/afs/flock.c
> +++ b/fs/afs/flock.c
> @@ -555,10 +555,6 @@ int afs_flock(struct file *file, int cmd, struct file_lock *fl)
>                 return -ENOLCK;
>
>         /* we're simulating flock() locks using posix locks on the server */
> -       fl->fl_owner = (fl_owner_t) file;
> -       fl->fl_start = 0;
> -       fl->fl_end = OFFSET_MAX;
> -
>         if (fl->fl_type == F_UNLCK)
>                 return afs_do_unlk(file, fl);
>         return afs_do_setlk(file, fl);
> diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c
> index d94ba0df9f4d..db8c1ca15d72 100644
> --- a/fs/ceph/locks.c
> +++ b/fs/ceph/locks.c
> @@ -52,10 +52,7 @@ static int ceph_lock_message(u8 lock_type, u16 operation, struct file *file,
>         else
>                 length = fl->fl_end - fl->fl_start + 1;
>
> -       if (lock_type == CEPH_LOCK_FCNTL)
> -               owner = secure_addr(fl->fl_owner);
> -       else
> -               owner = secure_addr(fl->fl_file);
> +       owner = secure_addr(fl->fl_owner);
>
>         dout("ceph_lock_message: rule: %d, op: %d, owner: %llx, pid: %llu, "
>              "start: %llu, length: %llu, wait: %d, type: %d", (int)lock_type,
> @@ -313,10 +310,7 @@ int lock_to_ceph_filelock(struct file_lock *lock,
>         cephlock->length = cpu_to_le64(lock->fl_end - lock->fl_start + 1);
>         cephlock->client = cpu_to_le64(0);
>         cephlock->pid = cpu_to_le64((u64)lock->fl_pid);
> -       if (lock->fl_flags & FL_POSIX)
> -               cephlock->owner = cpu_to_le64(secure_addr(lock->fl_owner));
> -       else
> -               cephlock->owner = cpu_to_le64(secure_addr(lock->fl_file));
> +       cephlock->owner = cpu_to_le64(secure_addr(lock->fl_owner));
>
>         switch (lock->fl_type) {
>         case F_RDLCK:
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 13f8bdec5110..0b25fec89558 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -2281,7 +2281,6 @@ static int fuse_file_flock(struct file *file, int cmd, struct file_lock *fl)
>                 struct fuse_file *ff = file->private_data;
>
>                 /* emulate flock with POSIX locks */
> -               fl->fl_owner = (fl_owner_t) file;
>                 ff->flock = true;
>                 err = fuse_setlk(file, fl, 1);
>         }
> diff --git a/fs/locks.c b/fs/locks.c
> index e663aeac579e..2129ba8ac062 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -322,6 +322,7 @@ static int flock_make_lock(struct file *filp, struct file_lock **lock,
>                 return -ENOMEM;
>
>         fl->fl_file = filp;
> +       fl->fl_owner = (fl_owner_t)filp;
>         fl->fl_pid = current->tgid;
>         fl->fl_flags = FL_FLOCK;
>         fl->fl_type = type;
> @@ -439,7 +440,7 @@ static int lease_init(struct file *filp, long type, struct file_lock *fl)
>         if (assign_type(fl, type) != 0)
>                 return -EINVAL;
>
> -       fl->fl_owner = current->files;
> +       fl->fl_owner = (fl_owner_t)filp;
>         fl->fl_pid = current->tgid;
>
>         fl->fl_file = filp;
> @@ -2304,6 +2305,7 @@ void locks_remove_file(struct file *filp)
>
>         if (filp->f_op->flock) {
>                 struct file_lock fl = {
> +                       .fl_owner = (fl_owner_t)filp,
>                         .fl_pid = current->tgid,
>                         .fl_file = filp,
>                         .fl_flags = FL_FLOCK,
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 284ca901fe16..c1edf7336315 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -916,10 +916,6 @@ int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>                 is_local = 1;
>
>         /* We're simulating flock() locks using posix locks on the server */
> -       fl->fl_owner = (fl_owner_t)filp;
> -       fl->fl_start = 0;
> -       fl->fl_end = OFFSET_MAX;
> -
>         if (fl->fl_type == F_UNLCK)
>                 return do_unlk(filp, cmd, fl, is_local);
>         return do_setlk(filp, cmd, fl, is_local);
> --
> 1.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-05-06 21:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-28 17:50 [PATCH] locks: ensure that fl_owner is always initialized properly in flock and lease codepaths Jeff Layton
2014-04-28 18:15 ` Greg Kroah-Hartman
2014-05-05 14:29 ` J. Bruce Fields
2014-05-06 21:45 ` Gregory Farnum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).