Re: [PATCH] ksys_mount: check for permissions before resource allocation

[Ilya Matveychikov <matvejchikov@gmail.com>]

> On Jun 6, 2018, at 6:22 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
> 
> Ilya Matveychikov <matvejchikov@gmail.com> writes:
> 
>>> On Jun 5, 2018, at 11:56 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>>> 
>>> Ilya Matveychikov <matvejchikov@gmail.com> writes:
>>> 
>>>> Just CC’ed to some of maintainers.
>>>> 
>>>> $ perl scripts/get_maintainer.pl fs/0001-ksys_mount-check-for-permissions-before-resource-all.patch
>>>> Alexander Viro <viro@zeniv.linux.org.uk> (maintainer:FILESYSTEMS (VFS and infrastructure))
>>>> linux-fsdevel@vger.kernel.org (open list:FILESYSTEMS (VFS and infrastructure))
>>>> linux-kernel@vger.kernel.org (open list)
>>>> 
>>>>> On Jun 5, 2018, at 6:00 AM, Ilya Matveychikov <matvejchikov@gmail.com> wrote:
>>>>> 
>>>>> Early check for mount permissions prevents possible allocation of 3
>>>>> pages from kmalloc() pool by unpriveledged user which can be used for
>>>>> spraying the kernel heap.
>>> 
>>> *Snort*
>>> 
>>> You clearly have not read may_mount.  Your modified code still
>>> let's unprivileged users in.  So even if all of Al's good objections
>>> were not applicable this change would still be buggy and wrong.
>>> 
>>> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>> 
>> 
>> Don’t get me wrong but may_mount() is:
>> 
>> static inline bool may_mount(void)
>> {
>>        return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN);
>> }
>> 
>> What do you mean by "You clearly have not read may_mount”? The only thing that
>> can affect may_mount result (as mentioned earlier) is that task’s NS capability
>> might be changed by security_sb_mount() hook.
>> 
>> So, do you think that is’s possible to NOT have CAP_SYS_ADMIN while entering to
>> ksys_mount() but getting it with the security_sb_mount() hook?
> 
> I mean it works for unprivileged users.
> 
> You can try "unshare -Urm" on a reasonably recent kernel and it will
> work and you can then mount and unmount things.
> 
> Strictly speaking it only works if you have the appropriate set of
> capabilities in your user namespace.  But that does not imply you are a
> privileged user in the broader sense.
> 
> Any user can create a user namespace, and become the root user
> in a user namespace.  The root user in a user namespace can create
> a mount namespace.  The root user in a user namespace can mount
> and unmount filesystems in their namespace.
> 
> Or in net anyone can call mount and get past the may_mount test.
> 
> Without reducing who can cause the kernel allocation moving the test is
> pointless.
> 

Ok, now I see. No reason to make change as it doesn’t really prevents users
of doing mount() using they own namespaces.

Thank you for the explanation.