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

[ebiederm@xmission.com (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