[RFC][PATCH] locks: Allow disabling mandatory locking at compile time

[RFC][PATCH] locks: Allow disabling mandatory locking at compile time

[Eric W. Biederman]

Mandatory locking appears to be almost unused and buggy and there
appears no real interest in doing anything with it.  Since effectively
no one uses the code and since the code is buggy let's allow it to be
disabled at compile time.  I would just suggest removing the code but
undoubtedly that will break some piece of userspace code somewhere.

For the distributions that don't care about this piece of code
this gives a nice starting point to make mandatory locking go away.

Cc: Benjamin Coddington <bcodding-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Dmitry Vyukov <dvyukov-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Cc: Jeff Layton <jeff.layton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
Cc: J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---

A piece of userspace software having problematic interactions with
mandatory locking recently came up as an issue and I am wondering if
there are enough people actually using and interested in mandatory
locking that it makes sense to push people to support it, or if
mandatory locking should be confined to it's own little corner of
existence where it can wither and die.

>From what little I can glean we want to discourage people from using
mandatory locking and to let it wither and die.  A Kconfig option that
allows mandatory locking to be disabled at compile time seems like the
first step in making that happen.  Perhaps in a decade or so when all
linux distributions are setting the option we can remove the code.

Does anyone know of any real world use cases of mandatory locking?

 fs/Kconfig         | 10 ++++++++
 fs/locks.c         |  2 ++
 fs/namespace.c     | 10 ++++++++
 include/linux/fs.h | 74 +++++++++++++++++++++++++++++-------------------------
 4 files changed, 62 insertions(+), 34 deletions(-)

diff --git a/fs/Kconfig b/fs/Kconfig
index da3f32f1a4e4..59322e6e76f4 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -67,6 +67,16 @@ config FILE_LOCKING
           for filesystems like NFS and for the flock() system
           call. Disabling this option saves about 11k.
 
+config MANDATORY_FILE_LOCKING
+	bool "Enable Mandatory file locking"
+	depends on FILE_LOCKING
+	default y
+	help
+	  This option enables files appropriately marked files on appropriely
+	  mounted filesystems to support mandatory locking.
+
+	  To the best of my knowledge this is dead code that no one cares about.
+
 source "fs/notify/Kconfig"
 
 source "fs/quota/Kconfig"
diff --git a/fs/locks.c b/fs/locks.c
index 0d2b3267e2a3..86c94674ab22 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1191,6 +1191,7 @@ static int posix_lock_inode_wait(struct inode *inode, struct file_lock *fl)
 	return error;
 }
 
+#ifdef CONFIG_MANDATORY_FILE_LOCKING
 /**
  * locks_mandatory_locked - Check for an active lock
  * @file: the file to check
@@ -1289,6 +1290,7 @@ int locks_mandatory_area(int read_write, struct inode *inode,
 }
 
 EXPORT_SYMBOL(locks_mandatory_area);
+#endif /* CONFIG_MANDATORY_FILE_LOCKING */
 
 static void lease_clear_pending(struct file_lock *fl, int arg)
 {
diff --git a/fs/namespace.c b/fs/namespace.c
index da70f7c4ece1..dee44b4791f0 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1584,6 +1584,14 @@ static inline bool may_mount(void)
 	return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN);
 }
 
+static inline bool may_mandlock(void)
+{
+#ifndef	CONFIG_MANDATORY_FILE_LOCKING
+	return false;
+#endif
+	return true;
+}
+
 /*
  * Now umount can handle mount points as well as block devices.
  * This is important for filesystems which use unnamed block devices.
@@ -2667,6 +2675,8 @@ long do_mount(const char *dev_name, const char __user *dir_name,
 				   type_page, flags, data_page);
 	if (!retval && !may_mount())
 		retval = -EPERM;
+	if (!retval && (flags & MS_MANDLOCK) && !may_mandlock())
+		retval = -EPERM;
 	if (retval)
 		goto dput_out;
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 08ae58dac726..e422714a0853 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2045,7 +2045,7 @@ extern struct kobject *fs_kobj;
 #define FLOCK_VERIFY_READ  1
 #define FLOCK_VERIFY_WRITE 2
 
-#ifdef CONFIG_FILE_LOCKING
+#ifdef CONFIG_MANDATORY_FILE_LOCKING
 extern int locks_mandatory_locked(struct file *);
 extern int locks_mandatory_area(int, struct inode *, struct file *, loff_t, size_t);
 
@@ -2090,6 +2090,45 @@ static inline int locks_verify_truncate(struct inode *inode,
 	return 0;
 }
 
+#else /* !CONFIG_MANDATORY_FILE_LOCKING */
+
+static inline int locks_mandatory_locked(struct file *file)
+{
+	return 0;
+}
+
+static inline int locks_mandatory_area(int rw, struct inode *inode,
+				       struct file *filp, loff_t offset,
+				       size_t count)
+{
+	return 0;
+}
+
+static inline int __mandatory_lock(struct inode *inode)
+{
+	return 0;
+}
+
+static inline int mandatory_lock(struct inode *inode)
+{
+	return 0;
+}
+
+static inline int locks_verify_locked(struct file *file)
+{
+	return 0;
+}
+
+static inline int locks_verify_truncate(struct inode *inode, struct file *filp,
+					size_t size)
+{
+	return 0;
+}
+
+#endif /* CONFIG_MANDATORY_FILE_LOCKING */
+
+
+#ifdef CONFIG_FILE_LOCKING
 static inline int break_lease(struct inode *inode, unsigned int mode)
 {
 	/*
@@ -2151,39 +2190,6 @@ static inline int break_layout(struct inode *inode, bool wait)
 }
 
 #else /* !CONFIG_FILE_LOCKING */
-static inline int locks_mandatory_locked(struct file *file)
-{
-	return 0;
-}
-
-static inline int locks_mandatory_area(int rw, struct inode *inode,
-				       struct file *filp, loff_t offset,
-				       size_t count)
-{
-	return 0;
-}
-
-static inline int __mandatory_lock(struct inode *inode)
-{
-	return 0;
-}
-
-static inline int mandatory_lock(struct inode *inode)
-{
-	return 0;
-}
-
-static inline int locks_verify_locked(struct file *file)
-{
-	return 0;
-}
-
-static inline int locks_verify_truncate(struct inode *inode, struct file *filp,
-					size_t size)
-{
-	return 0;
-}
-
 static inline int break_lease(struct inode *inode, unsigned int mode)
 {
 	return 0;
-- 
2.2.1

Re: [RFC][PATCH] locks: Allow disabling mandatory locking at compile time

[J. Bruce Fields]

On Wed, Nov 11, 2015 at 11:49:20AM -0600, Eric W. Biederman wrote:
> 
> Mandatory locking appears to be almost unused and buggy and there
> appears no real interest in doing anything with it.  Since effectively
> no one uses the code and since the code is buggy let's allow it to be
> disabled at compile time.  I would just suggest removing the code but
> undoubtedly that will break some piece of userspace code somewhere.
> 
> For the distributions that don't care about this piece of code
> this gives a nice starting point to make mandatory locking go away.
> 
> Cc: Benjamin Coddington <bcodding@redhat.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Jeff Layton <jeff.layton@primarydata.com>
> Cc: J. Bruce Fields <bfields@fieldses.org>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> 
> A piece of userspace software having problematic interactions with
> mandatory locking recently came up as an issue

Is there any more interesting story there?

> and I am wondering if there are enough people actually using and
> interested in mandatory locking that it makes sense to push people to
> support it, or if mandatory locking should be confined to it's own
> little corner of existence where it can wither and die.

I hate mandatory locking and would be delighted, but my opinion probably
shouldn't get too much weight.

> From what little I can glean we want to discourage people from using
> mandatory locking and to let it wither and die.  A Kconfig option that
> allows mandatory locking to be disabled at compile time seems like the
> first step in making that happen.  Perhaps in a decade or so when all
> linux distributions are setting the option we can remove the code.
> 
> Does anyone know of any real world use cases of mandatory locking?

Isn't byte-range locking on Windows mandatory?  So Samba people might be
the ones to talk to.  (Or Wine?  Or anyone else doing Windows
interoperability.)

My suspicion would be that the semantics they need are different enough
from what we support that we'd be better off ignoring it and starting
over from scratch anyway.  But I could be wrong.

--b.

Re: [RFC][PATCH] locks: Allow disabling mandatory locking at compile time

[Jeff Layton]

On Wed, 11 Nov 2015 15:26:07 -0500
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Wed, Nov 11, 2015 at 11:49:20AM -0600, Eric W. Biederman wrote:
> > 
> > Mandatory locking appears to be almost unused and buggy and there
> > appears no real interest in doing anything with it.  Since effectively
> > no one uses the code and since the code is buggy let's allow it to be
> > disabled at compile time.  I would just suggest removing the code but
> > undoubtedly that will break some piece of userspace code somewhere.
> > 
> > For the distributions that don't care about this piece of code
> > this gives a nice starting point to make mandatory locking go away.
> > 
> > Cc: Benjamin Coddington <bcodding@redhat.com>
> > Cc: Dmitry Vyukov <dvyukov@google.com>
> > Cc: Jeff Layton <jeff.layton@primarydata.com>
> > Cc: J. Bruce Fields <bfields@fieldses.org>
> > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> > ---
> > 
> > A piece of userspace software having problematic interactions with
> > mandatory locking recently came up as an issue
> 
> Is there any more interesting story there?
> 
> > and I am wondering if there are enough people actually using and
> > interested in mandatory locking that it makes sense to push people to
> > support it, or if mandatory locking should be confined to it's own
> > little corner of existence where it can wither and die.
> 
> I hate mandatory locking and would be delighted, but my opinion probably
> shouldn't get too much weight.
> 

Ditto...It's really hard to believe that anyone uses them, given the
well documented races in the Linux implementation.

> > From what little I can glean we want to discourage people from using
> > mandatory locking and to let it wither and die.  A Kconfig option that
> > allows mandatory locking to be disabled at compile time seems like the
> > first step in making that happen.  Perhaps in a decade or so when all
> > linux distributions are setting the option we can remove the code.
> > 
> > Does anyone know of any real world use cases of mandatory locking?
> 
> Isn't byte-range locking on Windows mandatory?  So Samba people might be
> the ones to talk to.  (Or Wine?  Or anyone else doing Windows
> interoperability.)
> 
> My suspicion would be that the semantics they need are different enough
> from what we support that we'd be better off ignoring it and starting
> over from scratch anyway.  But I could be wrong.
> 

Windows BRLs are mandatory but they have totally different semantics.

I think there is little reason to keep POSIX mandatory locks for
windows emulation purposes. I'm pretty sure Samba doesn't rely on them,
for instance, given that you have to use a funky mode bit combo to
enable them.

This patch seems like a logical step to me. I'll review it soon and
will plan to queue it up for v4.5 unless there are objections between
now and the next merge window.

Thanks!
-- 
Jeff Layton <jeff.layton@primarydata.com>

Re: [RFC][PATCH] locks: Allow disabling mandatory locking at compile time

[Jeremy Allison]

On Wed, Nov 11, 2015 at 05:44:01PM -0500, Jeff Layton wrote:
> Windows BRLs are mandatory but they have totally different semantics.
> 
> I think there is little reason to keep POSIX mandatory locks for
> windows emulation purposes. I'm pretty sure Samba doesn't rely on them,
> for instance, given that you have to use a funky mode bit combo to
> enable them.

Nope. We emulate Windows mandatory locks on top of POSIX advisory
locks. We don't use POSIX mandatory locks at all.

Re: [RFC][PATCH] locks: Allow disabling mandatory locking at compile time

[Eric W. Biederman]

Jeff Layton <jeff.layton@primarydata.com> writes:

> On Wed, 11 Nov 2015 15:26:07 -0500
> "J. Bruce Fields" <bfields@fieldses.org> wrote:
>
>> On Wed, Nov 11, 2015 at 11:49:20AM -0600, Eric W. Biederman wrote:
>> > 
>> > Mandatory locking appears to be almost unused and buggy and there
>> > appears no real interest in doing anything with it.  Since effectively
>> > no one uses the code and since the code is buggy let's allow it to be
>> > disabled at compile time.  I would just suggest removing the code but
>> > undoubtedly that will break some piece of userspace code somewhere.
>> > 
>> > For the distributions that don't care about this piece of code
>> > this gives a nice starting point to make mandatory locking go away.
>> > 
>> > Cc: Benjamin Coddington <bcodding@redhat.com>
>> > Cc: Dmitry Vyukov <dvyukov@google.com>
>> > Cc: Jeff Layton <jeff.layton@primarydata.com>
>> > Cc: J. Bruce Fields <bfields@fieldses.org>
>> > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
>> > ---
>> > 
>> > A piece of userspace software having problematic interactions with
>> > mandatory locking recently came up as an issue
>> 
>> Is there any more interesting story there?

Only that I overlooked them when implementing user namespace support for
mounting filesystems so it is currently possible to without privilege to
mount tmpfs with mandatory locking enabled and pass a file descriptor to
a daemon that was not expecting them.  Causing nice denial of service
attacks.

So I need to decide what to do with mandatory locking in user
namespaces.

As the consensus of this thread is that users of mandatory locking are
as rare as hen's teeth I can just not allow mandatory locking if you
something is being mounted just user namespace permissions.  If users
would be more wide spread I would need to figure out how to avoid breaking
users.

>> > and I am wondering if there are enough people actually using and
>> > interested in mandatory locking that it makes sense to push people to
>> > support it, or if mandatory locking should be confined to it's own
>> > little corner of existence where it can wither and die.
>> 
>> I hate mandatory locking and would be delighted, but my opinion probably
>> shouldn't get too much weight.
>> 
>
> Ditto...It's really hard to believe that anyone uses them, given the
> well documented races in the Linux implementation.

>> > From what little I can glean we want to discourage people from using
>> > mandatory locking and to let it wither and die.  A Kconfig option that
>> > allows mandatory locking to be disabled at compile time seems like the
>> > first step in making that happen.  Perhaps in a decade or so when all
>> > linux distributions are setting the option we can remove the code.
>> > 
>> > Does anyone know of any real world use cases of mandatory locking?
>> 
>> Isn't byte-range locking on Windows mandatory?  So Samba people might be
>> the ones to talk to.  (Or Wine?  Or anyone else doing Windows
>> interoperability.)
>> 
>> My suspicion would be that the semantics they need are different enough
>> from what we support that we'd be better off ignoring it and starting
>> over from scratch anyway.  But I could be wrong.
>> 
>
> Windows BRLs are mandatory but they have totally different semantics.
>
> I think there is little reason to keep POSIX mandatory locks for
> windows emulation purposes. I'm pretty sure Samba doesn't rely on them,
> for instance, given that you have to use a funky mode bit combo to
> enable them.
>
> This patch seems like a logical step to me. I'll review it soon and
> will plan to queue it up for v4.5 unless there are objections between
> now and the next merge window.

Thanks.

Given the general concensus of this thread it looks like we will also
want this incremental patch to deal with the user namespace case.

From: "Eric W. Biederman" <ebiederm@xmission.com>
Date: Wed, 11 Nov 2015 17:18:07 -0600
Subject: [PATCH] locks: Don't allow mounts in user namespaces to enable mandatory locking

Since no one uses mandatory locking and files with mandatory locks can
cause problems don't allow them in user namespaces.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 fs/namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index dee44b4791f0..95a349d5003d 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1589,7 +1589,7 @@ static inline bool may_mandlock(void)
 #ifndef	CONFIG_MANDATORY_FILE_LOCKING
 	return false;
 #endif
-	return true;
+	return capable(CAP_SYS_ADMIN);
 }
 
 /*
-- 
2.2.1

Re: [RFC][PATCH] locks: Allow disabling mandatory locking at compile time

[J. Bruce Fields]

On Wed, Nov 11, 2015 at 05:22:33PM -0600, Eric W. Biederman wrote:
> Jeff Layton <jeff.layton@primarydata.com> writes:
> 
> > On Wed, 11 Nov 2015 15:26:07 -0500
> > "J. Bruce Fields" <bfields@fieldses.org> wrote:
> >
> >> On Wed, Nov 11, 2015 at 11:49:20AM -0600, Eric W. Biederman wrote:
> >> > 
> >> > Mandatory locking appears to be almost unused and buggy and there
> >> > appears no real interest in doing anything with it.  Since effectively
> >> > no one uses the code and since the code is buggy let's allow it to be
> >> > disabled at compile time.  I would just suggest removing the code but
> >> > undoubtedly that will break some piece of userspace code somewhere.
> >> > 
> >> > For the distributions that don't care about this piece of code
> >> > this gives a nice starting point to make mandatory locking go away.
> >> > 
> >> > Cc: Benjamin Coddington <bcodding@redhat.com>
> >> > Cc: Dmitry Vyukov <dvyukov@google.com>
> >> > Cc: Jeff Layton <jeff.layton@primarydata.com>
> >> > Cc: J. Bruce Fields <bfields@fieldses.org>
> >> > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> >> > ---
> >> > 
> >> > A piece of userspace software having problematic interactions with
> >> > mandatory locking recently came up as an issue
> >> 
> >> Is there any more interesting story there?
> 
> Only that I overlooked them when implementing user namespace support for
> mounting filesystems so it is currently possible to without privilege to
> mount tmpfs with mandatory locking enabled and pass a file descriptor to
> a daemon that was not expecting them.  Causing nice denial of service
> attacks.
> 
> So I need to decide what to do with mandatory locking in user
> namespaces.
> 
> As the consensus of this thread is that users of mandatory locking are
> as rare as hen's teeth I can just not allow mandatory locking if you
> something is being mounted just user namespace permissions.

Sounds like a plan.  If nobody notices this limitation then that's
further evidence that we might be able to get away with deprecating it
eventually.

(Well, I wouldn't be surprised if there's some test suite somewhere that
includes a simple test for mandatory lock enforcement.  So, any user
other than that....)

--b.

Re: [RFC][PATCH] locks: Allow disabling mandatory locking at compile time

[Jeff Layton]

On Wed, 11 Nov 2015 20:33:11 -0500
"J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> wrote:

> On Wed, Nov 11, 2015 at 05:22:33PM -0600, Eric W. Biederman wrote:
> > Jeff Layton <jeff.layton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org> writes:
> > 
> > > On Wed, 11 Nov 2015 15:26:07 -0500
> > > "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> wrote:
> > >
> > >> On Wed, Nov 11, 2015 at 11:49:20AM -0600, Eric W. Biederman wrote:
> > >> > 
> > >> > Mandatory locking appears to be almost unused and buggy and there
> > >> > appears no real interest in doing anything with it.  Since effectively
> > >> > no one uses the code and since the code is buggy let's allow it to be
> > >> > disabled at compile time.  I would just suggest removing the code but
> > >> > undoubtedly that will break some piece of userspace code somewhere.
> > >> > 
> > >> > For the distributions that don't care about this piece of code
> > >> > this gives a nice starting point to make mandatory locking go away.
> > >> > 
> > >> > Cc: Benjamin Coddington <bcodding-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > >> > Cc: Dmitry Vyukov <dvyukov-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> > >> > Cc: Jeff Layton <jeff.layton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
> > >> > Cc: J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
> > >> > Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
> > >> > ---
> > >> > 
> > >> > A piece of userspace software having problematic interactions with
> > >> > mandatory locking recently came up as an issue
> > >> 
> > >> Is there any more interesting story there?
> > 
> > Only that I overlooked them when implementing user namespace support for
> > mounting filesystems so it is currently possible to without privilege to
> > mount tmpfs with mandatory locking enabled and pass a file descriptor to
> > a daemon that was not expecting them.  Causing nice denial of service
> > attacks.
> > 
> > So I need to decide what to do with mandatory locking in user
> > namespaces.
> > 
> > As the consensus of this thread is that users of mandatory locking are
> > as rare as hen's teeth I can just not allow mandatory locking if you
> > something is being mounted just user namespace permissions.
> 
> Sounds like a plan.  If nobody notices this limitation then that's
> further evidence that we might be able to get away with deprecating it
> eventually.
> 
> (Well, I wouldn't be surprised if there's some test suite somewhere that
> includes a simple test for mandatory lock enforcement.  So, any user
> other than that....)
> 

Yeah, ISTR that there are some (maybe in LTP)?

Still, we have all sorts of options to disable pieces of the kernel
these days for the tinification effort. This is just another.

Heck, we've already had CONFIG_FILE_LOCKING forever, so allowing people
to disable just mandatory locking seems harmless enough.

-- 
Jeff Layton <jeff.layton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>

Re: [RFC][PATCH] locks: Allow disabling mandatory locking at compile time

[Jeff Layton]

On Wed, 11 Nov 2015 17:22:33 -0600
ebiederm@xmission.com (Eric W. Biederman) wrote:

> Jeff Layton <jeff.layton@primarydata.com> writes:
> 
> > On Wed, 11 Nov 2015 15:26:07 -0500
> > "J. Bruce Fields" <bfields@fieldses.org> wrote:
> >
> >> On Wed, Nov 11, 2015 at 11:49:20AM -0600, Eric W. Biederman wrote:
> >> > 
> >> > Mandatory locking appears to be almost unused and buggy and there
> >> > appears no real interest in doing anything with it.  Since effectively
> >> > no one uses the code and since the code is buggy let's allow it to be
> >> > disabled at compile time.  I would just suggest removing the code but
> >> > undoubtedly that will break some piece of userspace code somewhere.
> >> > 
> >> > For the distributions that don't care about this piece of code
> >> > this gives a nice starting point to make mandatory locking go away.
> >> > 
> >> > Cc: Benjamin Coddington <bcodding@redhat.com>
> >> > Cc: Dmitry Vyukov <dvyukov@google.com>
> >> > Cc: Jeff Layton <jeff.layton@primarydata.com>
> >> > Cc: J. Bruce Fields <bfields@fieldses.org>
> >> > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> >> > ---
> >> > 
> >> > A piece of userspace software having problematic interactions with
> >> > mandatory locking recently came up as an issue
> >> 
> >> Is there any more interesting story there?
> 
> Only that I overlooked them when implementing user namespace support for
> mounting filesystems so it is currently possible to without privilege to
> mount tmpfs with mandatory locking enabled and pass a file descriptor to
> a daemon that was not expecting them.  Causing nice denial of service
> attacks.
> 
> So I need to decide what to do with mandatory locking in user
> namespaces.
> 
> As the consensus of this thread is that users of mandatory locking are
> as rare as hen's teeth I can just not allow mandatory locking if you
> something is being mounted just user namespace permissions.  If users
> would be more wide spread I would need to figure out how to avoid breaking
> users.
> 
> >> > and I am wondering if there are enough people actually using and
> >> > interested in mandatory locking that it makes sense to push people to
> >> > support it, or if mandatory locking should be confined to it's own
> >> > little corner of existence where it can wither and die.
> >> 
> >> I hate mandatory locking and would be delighted, but my opinion probably
> >> shouldn't get too much weight.
> >> 
> >
> > Ditto...It's really hard to believe that anyone uses them, given the
> > well documented races in the Linux implementation.
> 
> >> > From what little I can glean we want to discourage people from using
> >> > mandatory locking and to let it wither and die.  A Kconfig option that
> >> > allows mandatory locking to be disabled at compile time seems like the
> >> > first step in making that happen.  Perhaps in a decade or so when all
> >> > linux distributions are setting the option we can remove the code.
> >> > 
> >> > Does anyone know of any real world use cases of mandatory locking?
> >> 
> >> Isn't byte-range locking on Windows mandatory?  So Samba people might be
> >> the ones to talk to.  (Or Wine?  Or anyone else doing Windows
> >> interoperability.)
> >> 
> >> My suspicion would be that the semantics they need are different enough
> >> from what we support that we'd be better off ignoring it and starting
> >> over from scratch anyway.  But I could be wrong.
> >> 
> >
> > Windows BRLs are mandatory but they have totally different semantics.
> >
> > I think there is little reason to keep POSIX mandatory locks for
> > windows emulation purposes. I'm pretty sure Samba doesn't rely on them,
> > for instance, given that you have to use a funky mode bit combo to
> > enable them.
> >
> > This patch seems like a logical step to me. I'll review it soon and
> > will plan to queue it up for v4.5 unless there are objections between
> > now and the next merge window.
> 
> Thanks.
> 
> Given the general concensus of this thread it looks like we will also
> want this incremental patch to deal with the user namespace case.
> 
> From: "Eric W. Biederman" <ebiederm@xmission.com>
> Date: Wed, 11 Nov 2015 17:18:07 -0600
> Subject: [PATCH] locks: Don't allow mounts in user namespaces to enable mandatory locking
> 
> Since no one uses mandatory locking and files with mandatory locks can
> cause problems don't allow them in user namespaces.
> 
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>  fs/namespace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index dee44b4791f0..95a349d5003d 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -1589,7 +1589,7 @@ static inline bool may_mandlock(void)
>  #ifndef	CONFIG_MANDATORY_FILE_LOCKING
>  	return false;
>  #endif
> -	return true;
> +	return capable(CAP_SYS_ADMIN);
>  }
>  
>  /*

Fair enough. I'll merge that as well and we'll see what breaks.

-- 
Jeff Layton <jeff.layton@primarydata.com>

Re: [RFC][PATCH] locks: Allow disabling mandatory locking at compile time

[Eric W. Biederman]

Jeff Layton <jeff.layton@primarydata.com> writes:

>> Given the general concensus of this thread it looks like we will also
>> want this incremental patch to deal with the user namespace case.
>> 
>> From: "Eric W. Biederman" <ebiederm@xmission.com>
>> Date: Wed, 11 Nov 2015 17:18:07 -0600
>> Subject: [PATCH] locks: Don't allow mounts in user namespaces to enable mandatory locking
>> 
>> Since no one uses mandatory locking and files with mandatory locks can
>> cause problems don't allow them in user namespaces.
>> 
>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
>> ---
>>  fs/namespace.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/fs/namespace.c b/fs/namespace.c
>> index dee44b4791f0..95a349d5003d 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -1589,7 +1589,7 @@ static inline bool may_mandlock(void)
>>  #ifndef	CONFIG_MANDATORY_FILE_LOCKING
>>  	return false;
>>  #endif
>> -	return true;
>> +	return capable(CAP_SYS_ADMIN);
>>  }
>>  
>>  /*
>
> Fair enough. I'll merge that as well and we'll see what breaks.

Sounds good.  If something breaks just revert this bit.

The ways things work today is not an issue if people are actually using
mandatory file locking.


The fundamental issues that triggered my looking into this was a single
threaded server that accepts input from unprivileged users and hangs if
reading that input takes too long.  Mandatory locking just made it
easier to supply an input that hangs.

There are a countless other ways to make reading lots of data from a
file descriptor slow, the obvious ones being cpu load, disk load (for
local filesystems), and network load (for network filesystems).

Passing file descriptors appears to be the only path where mandatory
locking from files in a user namespace can affect processes outside of
the user namespace.  Given that there are many other ways to trigger a
slow file descriptor, that mandatory locking problems are easy to avoid,
I just can not see the existing kernel behavior as being wrong.

The current state of affairs is a problem in that we have a feature in
the kernel that apparently no one uses and everyone ignores.   Turning
off the feature is the right solution for that.

There is a privileged operation involved with enabling "-o mand" on
a filesystem.  The intent and the only value I can see in the "-o mand"
flag is that it determines if a filesystem will interpret the a specific
combinationation of file mode bits as specifying mandatory locking on a
file.  Allowing that on files accessible only to users inside of the user
namespace will not cause problems for the rest of the system except
potentially with passed file descriptors.

Eric