linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] locks: remove LOCK_MAND flock lock support
@ 2021-09-10 20:19 Jeff Layton
  2021-09-10 20:59 ` J. Bruce Fields
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Layton @ 2021-09-10 20:19 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: linux-kernel, bfields, viro, Matthew Wilcox, Stephen Rothwell

As best I can tell, the logic for these has been broken for a long time
(at least before the move to git), such that they never conflict with
anything. Also, nothing checks for these flags and prevented opens or
read/write behavior on the files. They don't seem to do anything.

Given that, we can rip these symbols out of the kernel, and just make
flock(2) return 0 when LOCK_MAND is set in order to preserve existing
behavior.

Cc: Matthew Wilcox <willy@infradead.org>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/locks.c                  |  3 ---
 fs/gfs2/file.c                   |  2 --
 fs/locks.c                       | 46 +++++++++++++++-----------------
 fs/nfs/file.c                    |  9 -------
 include/uapi/asm-generic/fcntl.h |  4 +++
 5 files changed, 25 insertions(+), 39 deletions(-)

Note that I do see some occurrences of LOCK_MAND in samba codebase, but
I think it's probably best that those are removed.

diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c
index bdeb271f47d9..d8c31069fbf2 100644
--- a/fs/ceph/locks.c
+++ b/fs/ceph/locks.c
@@ -302,9 +302,6 @@ int ceph_flock(struct file *file, int cmd, struct file_lock *fl)
 
 	if (!(fl->fl_flags & FL_FLOCK))
 		return -ENOLCK;
-	/* No mandatory locks */
-	if (fl->fl_type & LOCK_MAND)
-		return -EOPNOTSUPP;
 
 	dout("ceph_flock, fl_file: %p\n", fl->fl_file);
 
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index c559827cb6f9..078ef29e31bc 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -1338,8 +1338,6 @@ static int gfs2_flock(struct file *file, int cmd, struct file_lock *fl)
 {
 	if (!(fl->fl_flags & FL_FLOCK))
 		return -ENOLCK;
-	if (fl->fl_type & LOCK_MAND)
-		return -EOPNOTSUPP;
 
 	if (fl->fl_type == F_UNLCK) {
 		do_unflock(file, fl);
diff --git a/fs/locks.c b/fs/locks.c
index 3d6fb4ae847b..0e1d8a637e9c 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -461,8 +461,6 @@ static void locks_move_blocks(struct file_lock *new, struct file_lock *fl)
 }
 
 static inline int flock_translate_cmd(int cmd) {
-	if (cmd & LOCK_MAND)
-		return cmd & (LOCK_MAND | LOCK_RW);
 	switch (cmd) {
 	case LOCK_SH:
 		return F_RDLCK;
@@ -942,8 +940,6 @@ static bool flock_locks_conflict(struct file_lock *caller_fl,
 	 */
 	if (caller_fl->fl_file == sys_fl->fl_file)
 		return false;
-	if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND))
-		return false;
 
 	return locks_conflict(caller_fl, sys_fl);
 }
@@ -2116,11 +2112,9 @@ EXPORT_SYMBOL(locks_lock_inode_wait);
  *	- %LOCK_SH -- a shared lock.
  *	- %LOCK_EX -- an exclusive lock.
  *	- %LOCK_UN -- remove an existing lock.
- *	- %LOCK_MAND -- a 'mandatory' flock.
- *	  This exists to emulate Windows Share Modes.
+ *	- %LOCK_MAND -- a 'mandatory' flock. (DEPRECATED)
  *
- *	%LOCK_MAND can be combined with %LOCK_READ or %LOCK_WRITE to allow other
- *	processes read and write access respectively.
+ *	%LOCK_MAND support has been removed from the kernel.
  */
 SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
 {
@@ -2137,9 +2131,22 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
 	cmd &= ~LOCK_NB;
 	unlock = (cmd == LOCK_UN);
 
-	if (!unlock && !(cmd & LOCK_MAND) &&
-	    !(f.file->f_mode & (FMODE_READ|FMODE_WRITE)))
+	if (!unlock && !(f.file->f_mode & (FMODE_READ|FMODE_WRITE)))
+		goto out_putf;
+
+	/*
+	 * LOCK_MAND locks were broken for a long time in that they never
+	 * conflicted with one another and didn't prevent any sort of open,
+	 * read or write activity.
+	 *
+	 * Just ignore these requests now, to preserve legacy behavior, but
+	 * throw a warning to let people know that they don't actually work.
+	 */
+	if (cmd & LOCK_MAND) {
+		pr_warn_once("Attempt to set a LOCK_MAND lock via flock(2). This support has been removed and the request ignored.\n");
+		error = 0;
 		goto out_putf;
+	}
 
 	lock = flock_make_lock(f.file, cmd, NULL);
 	if (IS_ERR(lock)) {
@@ -2745,11 +2752,7 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 		seq_printf(f, " %s ",
 			     (inode == NULL) ? "*NOINODE*" : "ADVISORY ");
 	} else if (IS_FLOCK(fl)) {
-		if (fl->fl_type & LOCK_MAND) {
-			seq_puts(f, "FLOCK  MSNFS     ");
-		} else {
-			seq_puts(f, "FLOCK  ADVISORY  ");
-		}
+		seq_puts(f, "FLOCK  ADVISORY  ");
 	} else if (IS_LEASE(fl)) {
 		if (fl->fl_flags & FL_DELEG)
 			seq_puts(f, "DELEG  ");
@@ -2765,17 +2768,10 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 	} else {
 		seq_puts(f, "UNKNOWN UNKNOWN  ");
 	}
-	if (fl->fl_type & LOCK_MAND) {
-		seq_printf(f, "%s ",
-			       (fl->fl_type & LOCK_READ)
-			       ? (fl->fl_type & LOCK_WRITE) ? "RW   " : "READ "
-			       : (fl->fl_type & LOCK_WRITE) ? "WRITE" : "NONE ");
-	} else {
-		int type = IS_LEASE(fl) ? target_leasetype(fl) : fl->fl_type;
+	int type = IS_LEASE(fl) ? target_leasetype(fl) : fl->fl_type;
 
-		seq_printf(f, "%s ", (type == F_WRLCK) ? "WRITE" :
-				     (type == F_RDLCK) ? "READ" : "UNLCK");
-	}
+	seq_printf(f, "%s ", (type == F_WRLCK) ? "WRITE" :
+			     (type == F_RDLCK) ? "READ" : "UNLCK");
 	if (inode) {
 		/* userspace relies on this representation of dev_t */
 		seq_printf(f, "%d %02x:%02x:%lu ", fl_pid,
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index aa353fd58240..24e7dccce355 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -843,15 +843,6 @@ int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 	if (!(fl->fl_flags & FL_FLOCK))
 		return -ENOLCK;
 
-	/*
-	 * The NFSv4 protocol doesn't support LOCK_MAND, which is not part of
-	 * any standard. In principle we might be able to support LOCK_MAND
-	 * on NFSv2/3 since NLMv3/4 support DOS share modes, but for now the
-	 * NFS code is not set up for it.
-	 */
-	if (fl->fl_type & LOCK_MAND)
-		return -EINVAL;
-
 	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FLOCK)
 		is_local = 1;
 
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 9dc0bf0c5a6e..ecd0f5bdfc1d 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -181,6 +181,10 @@ struct f_owner_ex {
 				   blocking */
 #define LOCK_UN		8	/* remove lock */
 
+/*
+ * LOCK_MAND support has been removed from the kernel. We leave the symbols
+ * here to not break legacy builds, but these should not be used in new code.
+ */
 #define LOCK_MAND	32	/* This is a mandatory flock ... */
 #define LOCK_READ	64	/* which allows concurrent read operations */
 #define LOCK_WRITE	128	/* which allows concurrent write operations */
-- 
2.31.1


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

* Re: [PATCH] locks: remove LOCK_MAND flock lock support
  2021-09-10 20:19 [PATCH] locks: remove LOCK_MAND flock lock support Jeff Layton
@ 2021-09-10 20:59 ` J. Bruce Fields
  2021-09-10 22:12   ` Jeff Layton
  0 siblings, 1 reply; 3+ messages in thread
From: J. Bruce Fields @ 2021-09-10 20:59 UTC (permalink / raw)
  To: Jeff Layton
  Cc: linux-fsdevel, linux-kernel, viro, Matthew Wilcox, Stephen Rothwell

On Fri, Sep 10, 2021 at 04:19:15PM -0400, Jeff Layton wrote:
> As best I can tell, the logic for these has been broken for a long time
> (at least before the move to git), such that they never conflict with
> anything.

I've wondered about that!

But a grep of the Samba code shows it actually uses LOCK_MAND, why?
Looking closer now, I see that it sets LOCK_MAND in some cases but never
checks for LOCK_MAND, so there's absolutely no point unless the kernel
is doing something useful, which it isn't.  Huh.

Looking back at the kernel...  LOCK_MAND was introduced in Linux
2.4.0-test9pre6, and it was only checked in nfsd read and write code,
and only only on exports that had an "msnfs" export option set.

So it was a mandatory lock that only worked against NFS readers and
writers, and only if the admin knew to set this export option.

And, oh, look, I'd forgotten about this, but apparently in 2011 I
noticed that the msnfs option was totally undocumented and ripped it
out, in 9ce137eee4fe "nfsd: don't support msnfs export option".

I've heard no complaints since, so I guess that was an OK decision.

But I should have noticed at the same time that this also made LOCK_MAND
a no-op.

OK, sorry for the novel, and thanks for cleaning this up!

(Are you sending Samba a patch too?)

--b.

> Also, nothing checks for these flags and prevented opens or
> read/write behavior on the files. They don't seem to do anything.
> 
> Given that, we can rip these symbols out of the kernel, and just make
> flock(2) return 0 when LOCK_MAND is set in order to preserve existing
> behavior.
> 
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/locks.c                  |  3 ---
>  fs/gfs2/file.c                   |  2 --
>  fs/locks.c                       | 46 +++++++++++++++-----------------
>  fs/nfs/file.c                    |  9 -------
>  include/uapi/asm-generic/fcntl.h |  4 +++
>  5 files changed, 25 insertions(+), 39 deletions(-)
> 
> Note that I do see some occurrences of LOCK_MAND in samba codebase, but
> I think it's probably best that those are removed.
> 
> diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c
> index bdeb271f47d9..d8c31069fbf2 100644
> --- a/fs/ceph/locks.c
> +++ b/fs/ceph/locks.c
> @@ -302,9 +302,6 @@ int ceph_flock(struct file *file, int cmd, struct file_lock *fl)
>  
>  	if (!(fl->fl_flags & FL_FLOCK))
>  		return -ENOLCK;
> -	/* No mandatory locks */
> -	if (fl->fl_type & LOCK_MAND)
> -		return -EOPNOTSUPP;
>  
>  	dout("ceph_flock, fl_file: %p\n", fl->fl_file);
>  
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index c559827cb6f9..078ef29e31bc 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -1338,8 +1338,6 @@ static int gfs2_flock(struct file *file, int cmd, struct file_lock *fl)
>  {
>  	if (!(fl->fl_flags & FL_FLOCK))
>  		return -ENOLCK;
> -	if (fl->fl_type & LOCK_MAND)
> -		return -EOPNOTSUPP;
>  
>  	if (fl->fl_type == F_UNLCK) {
>  		do_unflock(file, fl);
> diff --git a/fs/locks.c b/fs/locks.c
> index 3d6fb4ae847b..0e1d8a637e9c 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -461,8 +461,6 @@ static void locks_move_blocks(struct file_lock *new, struct file_lock *fl)
>  }
>  
>  static inline int flock_translate_cmd(int cmd) {
> -	if (cmd & LOCK_MAND)
> -		return cmd & (LOCK_MAND | LOCK_RW);
>  	switch (cmd) {
>  	case LOCK_SH:
>  		return F_RDLCK;
> @@ -942,8 +940,6 @@ static bool flock_locks_conflict(struct file_lock *caller_fl,
>  	 */
>  	if (caller_fl->fl_file == sys_fl->fl_file)
>  		return false;
> -	if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND))
> -		return false;
>  
>  	return locks_conflict(caller_fl, sys_fl);
>  }
> @@ -2116,11 +2112,9 @@ EXPORT_SYMBOL(locks_lock_inode_wait);
>   *	- %LOCK_SH -- a shared lock.
>   *	- %LOCK_EX -- an exclusive lock.
>   *	- %LOCK_UN -- remove an existing lock.
> - *	- %LOCK_MAND -- a 'mandatory' flock.
> - *	  This exists to emulate Windows Share Modes.
> + *	- %LOCK_MAND -- a 'mandatory' flock. (DEPRECATED)
>   *
> - *	%LOCK_MAND can be combined with %LOCK_READ or %LOCK_WRITE to allow other
> - *	processes read and write access respectively.
> + *	%LOCK_MAND support has been removed from the kernel.
>   */
>  SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
>  {
> @@ -2137,9 +2131,22 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
>  	cmd &= ~LOCK_NB;
>  	unlock = (cmd == LOCK_UN);
>  
> -	if (!unlock && !(cmd & LOCK_MAND) &&
> -	    !(f.file->f_mode & (FMODE_READ|FMODE_WRITE)))
> +	if (!unlock && !(f.file->f_mode & (FMODE_READ|FMODE_WRITE)))
> +		goto out_putf;
> +
> +	/*
> +	 * LOCK_MAND locks were broken for a long time in that they never
> +	 * conflicted with one another and didn't prevent any sort of open,
> +	 * read or write activity.
> +	 *
> +	 * Just ignore these requests now, to preserve legacy behavior, but
> +	 * throw a warning to let people know that they don't actually work.
> +	 */
> +	if (cmd & LOCK_MAND) {
> +		pr_warn_once("Attempt to set a LOCK_MAND lock via flock(2). This support has been removed and the request ignored.\n");
> +		error = 0;
>  		goto out_putf;
> +	}
>  
>  	lock = flock_make_lock(f.file, cmd, NULL);
>  	if (IS_ERR(lock)) {
> @@ -2745,11 +2752,7 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
>  		seq_printf(f, " %s ",
>  			     (inode == NULL) ? "*NOINODE*" : "ADVISORY ");
>  	} else if (IS_FLOCK(fl)) {
> -		if (fl->fl_type & LOCK_MAND) {
> -			seq_puts(f, "FLOCK  MSNFS     ");
> -		} else {
> -			seq_puts(f, "FLOCK  ADVISORY  ");
> -		}
> +		seq_puts(f, "FLOCK  ADVISORY  ");
>  	} else if (IS_LEASE(fl)) {
>  		if (fl->fl_flags & FL_DELEG)
>  			seq_puts(f, "DELEG  ");
> @@ -2765,17 +2768,10 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
>  	} else {
>  		seq_puts(f, "UNKNOWN UNKNOWN  ");
>  	}
> -	if (fl->fl_type & LOCK_MAND) {
> -		seq_printf(f, "%s ",
> -			       (fl->fl_type & LOCK_READ)
> -			       ? (fl->fl_type & LOCK_WRITE) ? "RW   " : "READ "
> -			       : (fl->fl_type & LOCK_WRITE) ? "WRITE" : "NONE ");
> -	} else {
> -		int type = IS_LEASE(fl) ? target_leasetype(fl) : fl->fl_type;
> +	int type = IS_LEASE(fl) ? target_leasetype(fl) : fl->fl_type;
>  
> -		seq_printf(f, "%s ", (type == F_WRLCK) ? "WRITE" :
> -				     (type == F_RDLCK) ? "READ" : "UNLCK");
> -	}
> +	seq_printf(f, "%s ", (type == F_WRLCK) ? "WRITE" :
> +			     (type == F_RDLCK) ? "READ" : "UNLCK");
>  	if (inode) {
>  		/* userspace relies on this representation of dev_t */
>  		seq_printf(f, "%d %02x:%02x:%lu ", fl_pid,
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index aa353fd58240..24e7dccce355 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -843,15 +843,6 @@ int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>  	if (!(fl->fl_flags & FL_FLOCK))
>  		return -ENOLCK;
>  
> -	/*
> -	 * The NFSv4 protocol doesn't support LOCK_MAND, which is not part of
> -	 * any standard. In principle we might be able to support LOCK_MAND
> -	 * on NFSv2/3 since NLMv3/4 support DOS share modes, but for now the
> -	 * NFS code is not set up for it.
> -	 */
> -	if (fl->fl_type & LOCK_MAND)
> -		return -EINVAL;
> -
>  	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FLOCK)
>  		is_local = 1;
>  
> diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
> index 9dc0bf0c5a6e..ecd0f5bdfc1d 100644
> --- a/include/uapi/asm-generic/fcntl.h
> +++ b/include/uapi/asm-generic/fcntl.h
> @@ -181,6 +181,10 @@ struct f_owner_ex {
>  				   blocking */
>  #define LOCK_UN		8	/* remove lock */
>  
> +/*
> + * LOCK_MAND support has been removed from the kernel. We leave the symbols
> + * here to not break legacy builds, but these should not be used in new code.
> + */
>  #define LOCK_MAND	32	/* This is a mandatory flock ... */
>  #define LOCK_READ	64	/* which allows concurrent read operations */
>  #define LOCK_WRITE	128	/* which allows concurrent write operations */
> -- 
> 2.31.1

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

* Re: [PATCH] locks: remove LOCK_MAND flock lock support
  2021-09-10 20:59 ` J. Bruce Fields
@ 2021-09-10 22:12   ` Jeff Layton
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff Layton @ 2021-09-10 22:12 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: linux-fsdevel, linux-kernel, viro, Matthew Wilcox, Stephen Rothwell

On Fri, 2021-09-10 at 16:59 -0400, J. Bruce Fields wrote:
> On Fri, Sep 10, 2021 at 04:19:15PM -0400, Jeff Layton wrote:
> > As best I can tell, the logic for these has been broken for a long time
> > (at least before the move to git), such that they never conflict with
> > anything.
> 
> I've wondered about that!
> 
> But a grep of the Samba code shows it actually uses LOCK_MAND, why?
> Looking closer now, I see that it sets LOCK_MAND in some cases but never
> checks for LOCK_MAND, so there's absolutely no point unless the kernel
> is doing something useful, which it isn't.  Huh.
> 
> Looking back at the kernel...  LOCK_MAND was introduced in Linux
> 2.4.0-test9pre6, and it was only checked in nfsd read and write code,
> and only only on exports that had an "msnfs" export option set.
> 
> So it was a mandatory lock that only worked against NFS readers and
> writers, and only if the admin knew to set this export option.
> 
> And, oh, look, I'd forgotten about this, but apparently in 2011 I
> noticed that the msnfs option was totally undocumented and ripped it
> out, in 9ce137eee4fe "nfsd: don't support msnfs export option".
> 
> I've heard no complaints since, so I guess that was an OK decision.
> 
> But I should have noticed at the same time that this also made LOCK_MAND
> a no-op.
> 
> OK, sorry for the novel, and thanks for cleaning this up!
> 
> (Are you sending Samba a patch too?)
> 
> --b.
> 

Ahh, thanks for the archaeology! That makes sense. I took a look at
Samba, but haven't cooked up a patch for it yet.

After taking a look, I sort of wonder whether there might be out of tree
filesystems (GPFS?) that do support these flags properly. OTOH, I'm not
inclined to worry about them too much. Maybe if they want to chime in we
can work with them to ease the transition.

I'll probably do a patch for samba soon, just to kick off the discussion
with those guys. I'll see what I can come up with.

> > Also, nothing checks for these flags and prevented opens or
> > read/write behavior on the files. They don't seem to do anything.
> > 
> > Given that, we can rip these symbols out of the kernel, and just make
> > flock(2) return 0 when LOCK_MAND is set in order to preserve existing
> > behavior.
> > 
> > Cc: Matthew Wilcox <willy@infradead.org>
> > Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/ceph/locks.c                  |  3 ---
> >  fs/gfs2/file.c                   |  2 --
> >  fs/locks.c                       | 46 +++++++++++++++-----------------
> >  fs/nfs/file.c                    |  9 -------
> >  include/uapi/asm-generic/fcntl.h |  4 +++
> >  5 files changed, 25 insertions(+), 39 deletions(-)
> > 
> > Note that I do see some occurrences of LOCK_MAND in samba codebase, but
> > I think it's probably best that those are removed.
> > 
> > diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c
> > index bdeb271f47d9..d8c31069fbf2 100644
> > --- a/fs/ceph/locks.c
> > +++ b/fs/ceph/locks.c
> > @@ -302,9 +302,6 @@ int ceph_flock(struct file *file, int cmd, struct file_lock *fl)
> >  
> >  	if (!(fl->fl_flags & FL_FLOCK))
> >  		return -ENOLCK;
> > -	/* No mandatory locks */
> > -	if (fl->fl_type & LOCK_MAND)
> > -		return -EOPNOTSUPP;
> >  
> >  	dout("ceph_flock, fl_file: %p\n", fl->fl_file);
> >  
> > diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> > index c559827cb6f9..078ef29e31bc 100644
> > --- a/fs/gfs2/file.c
> > +++ b/fs/gfs2/file.c
> > @@ -1338,8 +1338,6 @@ static int gfs2_flock(struct file *file, int cmd, struct file_lock *fl)
> >  {
> >  	if (!(fl->fl_flags & FL_FLOCK))
> >  		return -ENOLCK;
> > -	if (fl->fl_type & LOCK_MAND)
> > -		return -EOPNOTSUPP;
> >  
> >  	if (fl->fl_type == F_UNLCK) {
> >  		do_unflock(file, fl);
> > diff --git a/fs/locks.c b/fs/locks.c
> > index 3d6fb4ae847b..0e1d8a637e9c 100644
> > --- a/fs/locks.c
> > +++ b/fs/locks.c
> > @@ -461,8 +461,6 @@ static void locks_move_blocks(struct file_lock *new, struct file_lock *fl)
> >  }
> >  
> >  static inline int flock_translate_cmd(int cmd) {
> > -	if (cmd & LOCK_MAND)
> > -		return cmd & (LOCK_MAND | LOCK_RW);
> >  	switch (cmd) {
> >  	case LOCK_SH:
> >  		return F_RDLCK;
> > @@ -942,8 +940,6 @@ static bool flock_locks_conflict(struct file_lock *caller_fl,
> >  	 */
> >  	if (caller_fl->fl_file == sys_fl->fl_file)
> >  		return false;
> > -	if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND))
> > -		return false;
> >  
> >  	return locks_conflict(caller_fl, sys_fl);
> >  }
> > @@ -2116,11 +2112,9 @@ EXPORT_SYMBOL(locks_lock_inode_wait);
> >   *	- %LOCK_SH -- a shared lock.
> >   *	- %LOCK_EX -- an exclusive lock.
> >   *	- %LOCK_UN -- remove an existing lock.
> > - *	- %LOCK_MAND -- a 'mandatory' flock.
> > - *	  This exists to emulate Windows Share Modes.
> > + *	- %LOCK_MAND -- a 'mandatory' flock. (DEPRECATED)
> >   *
> > - *	%LOCK_MAND can be combined with %LOCK_READ or %LOCK_WRITE to allow other
> > - *	processes read and write access respectively.
> > + *	%LOCK_MAND support has been removed from the kernel.
> >   */
> >  SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
> >  {
> > @@ -2137,9 +2131,22 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
> >  	cmd &= ~LOCK_NB;
> >  	unlock = (cmd == LOCK_UN);
> >  
> > -	if (!unlock && !(cmd & LOCK_MAND) &&
> > -	    !(f.file->f_mode & (FMODE_READ|FMODE_WRITE)))
> > +	if (!unlock && !(f.file->f_mode & (FMODE_READ|FMODE_WRITE)))
> > +		goto out_putf;
> > +
> > +	/*
> > +	 * LOCK_MAND locks were broken for a long time in that they never
> > +	 * conflicted with one another and didn't prevent any sort of open,
> > +	 * read or write activity.
> > +	 *
> > +	 * Just ignore these requests now, to preserve legacy behavior, but
> > +	 * throw a warning to let people know that they don't actually work.
> > +	 */
> > +	if (cmd & LOCK_MAND) {
> > +		pr_warn_once("Attempt to set a LOCK_MAND lock via flock(2). This support has been removed and the request ignored.\n");
> > +		error = 0;
> >  		goto out_putf;
> > +	}
> >  
> >  	lock = flock_make_lock(f.file, cmd, NULL);
> >  	if (IS_ERR(lock)) {
> > @@ -2745,11 +2752,7 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
> >  		seq_printf(f, " %s ",
> >  			     (inode == NULL) ? "*NOINODE*" : "ADVISORY ");
> >  	} else if (IS_FLOCK(fl)) {
> > -		if (fl->fl_type & LOCK_MAND) {
> > -			seq_puts(f, "FLOCK  MSNFS     ");
> > -		} else {
> > -			seq_puts(f, "FLOCK  ADVISORY  ");
> > -		}
> > +		seq_puts(f, "FLOCK  ADVISORY  ");
> >  	} else if (IS_LEASE(fl)) {
> >  		if (fl->fl_flags & FL_DELEG)
> >  			seq_puts(f, "DELEG  ");
> > @@ -2765,17 +2768,10 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
> >  	} else {
> >  		seq_puts(f, "UNKNOWN UNKNOWN  ");
> >  	}
> > -	if (fl->fl_type & LOCK_MAND) {
> > -		seq_printf(f, "%s ",
> > -			       (fl->fl_type & LOCK_READ)
> > -			       ? (fl->fl_type & LOCK_WRITE) ? "RW   " : "READ "
> > -			       : (fl->fl_type & LOCK_WRITE) ? "WRITE" : "NONE ");
> > -	} else {
> > -		int type = IS_LEASE(fl) ? target_leasetype(fl) : fl->fl_type;
> > +	int type = IS_LEASE(fl) ? target_leasetype(fl) : fl->fl_type;
> >  
> > -		seq_printf(f, "%s ", (type == F_WRLCK) ? "WRITE" :
> > -				     (type == F_RDLCK) ? "READ" : "UNLCK");
> > -	}
> > +	seq_printf(f, "%s ", (type == F_WRLCK) ? "WRITE" :
> > +			     (type == F_RDLCK) ? "READ" : "UNLCK");
> >  	if (inode) {
> >  		/* userspace relies on this representation of dev_t */
> >  		seq_printf(f, "%d %02x:%02x:%lu ", fl_pid,
> > diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> > index aa353fd58240..24e7dccce355 100644
> > --- a/fs/nfs/file.c
> > +++ b/fs/nfs/file.c
> > @@ -843,15 +843,6 @@ int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
> >  	if (!(fl->fl_flags & FL_FLOCK))
> >  		return -ENOLCK;
> >  
> > -	/*
> > -	 * The NFSv4 protocol doesn't support LOCK_MAND, which is not part of
> > -	 * any standard. In principle we might be able to support LOCK_MAND
> > -	 * on NFSv2/3 since NLMv3/4 support DOS share modes, but for now the
> > -	 * NFS code is not set up for it.
> > -	 */
> > -	if (fl->fl_type & LOCK_MAND)
> > -		return -EINVAL;
> > -
> >  	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FLOCK)
> >  		is_local = 1;
> >  
> > diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
> > index 9dc0bf0c5a6e..ecd0f5bdfc1d 100644
> > --- a/include/uapi/asm-generic/fcntl.h
> > +++ b/include/uapi/asm-generic/fcntl.h
> > @@ -181,6 +181,10 @@ struct f_owner_ex {
> >  				   blocking */
> >  #define LOCK_UN		8	/* remove lock */
> >  
> > +/*
> > + * LOCK_MAND support has been removed from the kernel. We leave the symbols
> > + * here to not break legacy builds, but these should not be used in new code.
> > + */
> >  #define LOCK_MAND	32	/* This is a mandatory flock ... */
> >  #define LOCK_READ	64	/* which allows concurrent read operations */
> >  #define LOCK_WRITE	128	/* which allows concurrent write operations */
> > -- 
> > 2.31.1

-- 
Jeff Layton <jlayton@kernel.org>


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

end of thread, other threads:[~2021-09-10 22:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-10 20:19 [PATCH] locks: remove LOCK_MAND flock lock support Jeff Layton
2021-09-10 20:59 ` J. Bruce Fields
2021-09-10 22:12   ` Jeff Layton

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).