linux-kernel - Re: Sealed memfd & no-fault mmap

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]

Date:   Sat, 29 May 2021 05:44:01 -1000
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     "Lin, Ming" <minggr@...il.com>
Cc:     Hugh Dickins <hughd@...gle.com>, Simon Ser <contact@...rsion.fr>,
        Peter Xu <peterx@...hat.com>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        Matthew Wilcox <willy@...radead.org>,
        Dan Williams <dan.j.williams@...el.com>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Will Deacon <will@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        David Herrmann <dh.herrmann@...il.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Greg Kroah-Hartman <greg@...ah.com>,
        "tytso@....edu" <tytso@....edu>
Subject: Re: Sealed memfd & no-fault mmap

On Fri, May 28, 2021 at 9:31 PM Lin, Ming <minggr@...il.com> wrote:
>
> I should check the vma is not writable.
>
> -               if (!IS_NOFAULT(inode))
> +               if (!IS_NOFAULT(inode) || (vma->vm_flags & VM_WRITE))
>                          return -EINVAL;

That might be enough, yes.

But if this is sufficient for the compositor needs, and the rule is
that this only works for read-only mappings, then I think the flag in
the inode becomes the wrong thing to do.

Because if it's a read-only mapping, and we only ever care about
inserting zero pages into the page tables - and they never become part
of the shared memory region itself, then it really is purely about
that mmap, not about the shm inode.

So then it really does become purely about one particular mmap, and it
really should be a "madvise()" issue, not a "mark inode as no-fault".

I'd almost be inclined to just add a new "flags" field to the vma.
We've been running out of vma flags for a long time, to the point that
some of them are only available on 64-bit architectures.

I get the feeling that we should just bite the bullet and make
"vm_flags" be an u64. Or possibly make it two explicitly 32-bit
entities (vm_flags and vm_extra). Get rid of the silly 64-bit-only
"high" flags, and get rid of our artificial "we don't have enough
bits".

Because we already in practice *do* have enough bits, we've just
artificially limited ourselves to "on 32-bit architectures we only
have 32 bits in that field".

But all of this is very much dependent on that "this NOFAULT case
really only works for reads, not for writes".

(Alternatively, we could allow the *mapping* itself to be writable,
but always fault on writes, and only insert a read-only zero page)

              Linus