linux-hardening - Re: [RFC PATCH v2 7/8] mseal:Check seal flag for mmap(2)

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: Tue, 17 Oct 2023 10:04:54 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: jeffxu@...omium.org
Cc: akpm@...ux-foundation.org, keescook@...omium.org, jannh@...gle.com, 
	sroettger@...gle.com, willy@...radead.org, gregkh@...uxfoundation.org, 
	jeffxu@...gle.com, jorgelo@...omium.org, groeck@...omium.org, 
	linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org, 
	linux-mm@...ck.org, surenb@...gle.com, alex.sierra@....com, 
	apopple@...dia.com, aneesh.kumar@...ux.ibm.com, axelrasmussen@...gle.com, 
	ben@...adent.org.uk, catalin.marinas@....com, david@...hat.com, 
	dwmw@...zon.co.uk, ying.huang@...el.com, hughd@...gle.com, joey.gouly@....com, 
	corbet@....net, wangkefeng.wang@...wei.com, Liam.Howlett@...cle.com, 
	lstoakes@...il.com, mawupeng1@...wei.com, linmiaohe@...wei.com, 
	namit@...are.com, peterx@...hat.com, peterz@...radead.org, 
	ryan.roberts@....com, shr@...kernel.io, vbabka@...e.cz, 
	xiujianfeng@...wei.com, yu.ma@...el.com, zhangpeng362@...wei.com, 
	dave.hansen@...el.com, luto@...nel.org, linux-hardening@...r.kernel.org
Subject: Re: [RFC PATCH v2 7/8] mseal:Check seal flag for mmap(2)

On Tue, 17 Oct 2023 at 02:08, <jeffxu@...omium.org> wrote:
>
> Note: Of all the call paths that goes into do_mmap(),
> ksys_mmap_pgoff() is the only place where
> checkSeals = MM_SEAL_MMAP. The rest has checkSeals = 0.

Again, this is all completely nonsensical.

First off, since seals only exist on existing vma's, the "MMAP seal"
makes no sense to begin with. You cannot mmap twice - and mmap'ing
over an existing mapping involves unmapping the old one.

So a "mmap seal" is nonsensical. What should protect a mapping is "you
cannot unmap this". That automatically means that you cannot mmap over
it.

In other words, all these sealing flag semantics are completely random
noise. None of this makes sense.

You need to EXPLAIN what the concept is.

Honestly, there is only two kinds of sealing that makes sense:

 - you cannot change the permissions of some area

 - you cannot unmap an area

where that first version might then have sub-cases (maybe you can make
permissions _stricter_, but not the other way)?

And dammit, once something is sealed, it is SEALED. None of this crazy
"one place honors the sealing, random other places do not".

I do *NOT* want to see another random patch series tomorrow that
modifies something small here.

I want to get an EXPLANATION and the whole "what the f*ck is the
concept". No more random rules. No more nonsensical code. No more of
this "one place honors seals, another one does not".

Seriously. As long as this is chock-full of these kinds of random
"this makes no sense", please don't send any patches AT ALL. Explain
the high-level rules first, and if you cannot explain why one random
place does something different from all the other random places, don't
even bother.

No more random code. No more random seals. None of this crazy "you
ostensibly can't unmap a vma, but you you can actually unmap it by
mmap'ing over it and then unmapping the new one".

             Linus