linux-kernel - Re: [PATCH v10 0/5] Introduce mseal

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: <d46qb2rkfsagw22u6ishgagsvsmqsu5nrmf5up5mhi6xrwolyt@6ir6g2v63of7>
Date: Tue, 14 May 2024 17:28:06 -0400
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: jeffxu@...omium.org, keescook@...omium.org, jannh@...gle.com,
        sroettger@...gle.com, willy@...radead.org, gregkh@...uxfoundation.org,
        torvalds@...ux-foundation.org, usama.anjum@...labora.com,
        corbet@....net, surenb@...gle.com, merimus@...gle.com,
        rdunlap@...radead.org, jeffxu@...gle.com, jorgelo@...omium.org,
        groeck@...omium.org, linux-kernel@...r.kernel.org,
        linux-kselftest@...r.kernel.org, linux-mm@...ck.org,
        pedro.falcato@...il.com, dave.hansen@...el.com,
        linux-hardening@...r.kernel.org, deraadt@...nbsd.org
Subject: Re: [PATCH v10 0/5] Introduce mseal

* Andrew Morton <akpm@...ux-foundation.org> [240514 13:47]:
> On Mon, 15 Apr 2024 16:35:19 +0000 jeffxu@...omium.org wrote:
> 
> > This patchset proposes a new mseal() syscall for the Linux kernel.
> 
> I have not moved this into mm-stable for a 6.10 merge.  Mainly because
> of the total lack of Reviewed-by:s and Acked-by:s.
> 
> The code appears to be stable enough for a merge.
> 
> It's awkward that we're in conference this week, but I ask people to
> give consideration to the desirability of moving mseal() into mainline
> sometime over the next week, please.

I have looked at this code a fair bit at this point, but I wanted to get
some clarification on the fact that we now have mseal doing checks
upfront while others fail in the middle.

mbind:
                /*
                 * If any vma in the range got policy other than MPOL_BIND                                                             
                 * or MPOL_PREFERRED_MANY we return error. We don't reset                                                              
                 * the home node for vmas we already updated before.
                 */ 


mlock:
mlock will abort (through one path), when it sees a gap in mapped areas,
but will not undo what it did so far.

mlock_fixup can fail on vma_modify_flags(), but previous vmas are not
updated.  This can fail due to allocation failures on the splitting of
VMAs (or failed merging).  The allocations could happen before, but this
is more work (but doable, no doubt).

userfaultfd is... complicated.

And even munmap() can fail and NOT undo the previous split(s).
mmap.c:
                        /*
                         * If userfaultfd_unmap_prep returns an error the vmas                                                         
                         * will remain split, but userland will get a                                                                  
                         * highly unexpected error anyway. This is no
                         * different than the case where the first of the two                                                          
                         * __split_vma fails, but we don't undo the first
                         * split, despite we could. This is unlikely enough                                                            
                         * failure that it's not worth optimizing it for.                                                              
                         */


But this one is different form the others..
madvise:
        /*
         * If the interval [start,end) covers some unmapped address                                                                    
         * ranges, just ignore them, but return -ENOMEM at the end.
         * - different from the way of handling in mlock etc.
         */

Either we are planning to clean this up and do what we can up-front, or
just move the mseal check with the rest.  Otherwise we are making a
larger mess with more technical dept for a single user, and I think this
is not an acceptable trade-off.

Considering the benchmarks that were provided, performance arguments
seem like they are not a concern.

I want to know if we are planning to sort and move existing checks if we
proceed with this change?

Thanks,
Liam