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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <64021AF2-81EE-439C-91D4-9A33AB0D08F7@gmail.com>
Date:   Wed, 6 Jun 2018 13:32:12 +0400
From:   Ilya Matveychikov <matvejchikov@...il.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     linux-kernel@...r.kernel.org,
        Alexander Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] ksys_mount: check for permissions before resource
 allocation



> On Jun 5, 2018, at 11:56 PM, Eric W. Biederman <ebiederm@...ssion.com> wrote:
> 
> Ilya Matveychikov <matvejchikov@...il.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@...iv.linux.org.uk> (maintainer:FILESYSTEMS (VFS and infrastructure))
>> linux-fsdevel@...r.kernel.org (open list:FILESYSTEMS (VFS and infrastructure))
>> linux-kernel@...r.kernel.org (open list)
>> 
>>> On Jun 5, 2018, at 6:00 AM, Ilya Matveychikov <matvejchikov@...il.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@...ssion.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?

This is the only case I see that using may_mount() before security_sb_mount()
is wrong. This was the point?