linux-kernel - Re: [RFC PATCH v5 1/7] mseal, system mappings: kernel config and header change

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: <CABi2SkX3o-PdeuzownVkFT-oo8tjtaaS9ebOALu+r6O1S6W3sg@mail.gmail.com>
Date: Thu, 13 Feb 2025 14:00:22 -0800
From: Jeff Xu <jeffxu@...omium.org>
To: "Liam R. Howlett" <Liam.Howlett@...cle.com>, Kees Cook <kees@...nel.org>, 
	Jeff Xu <jeffxu@...omium.org>, akpm@...ux-foundation.org, jannh@...gle.com, 
	torvalds@...ux-foundation.org, vbabka@...e.cz, lorenzo.stoakes@...cle.com, 
	adhemerval.zanella@...aro.org, oleg@...hat.com, avagin@...il.com, 
	benjamin@...solutions.net, linux-kernel@...r.kernel.org, 
	linux-hardening@...r.kernel.org, linux-mm@...ck.org, jorgelo@...omium.org, 
	sroettger@...gle.com, hch@....de, ojeda@...nel.org, 
	thomas.weissschuh@...utronix.de, adobriyan@...il.com, 
	johannes@...solutions.net, pedro.falcato@...il.com, hca@...ux.ibm.com, 
	willy@...radead.org, anna-maria@...utronix.de, mark.rutland@....com, 
	linus.walleij@...aro.org, Jason@...c4.com, deller@....de, 
	rdunlap@...radead.org, davem@...emloft.net, peterx@...hat.com, 
	f.fainelli@...il.com, gerg@...nel.org, dave.hansen@...ux.intel.com, 
	mingo@...nel.org, ardb@...nel.org, mhocko@...e.com, 42.hyeyoo@...il.com, 
	peterz@...radead.org, ardb@...gle.com, enh@...gle.com, rientjes@...gle.com, 
	groeck@...omium.org, mpe@...erman.id.au, aleksandr.mikhalitsyn@...onical.com, 
	mike.rapoport@...il.com
Subject: Re: [RFC PATCH v5 1/7] mseal, system mappings: kernel config and
 header change

On Thu, Feb 13, 2025 at 12:54 PM Liam R. Howlett
<Liam.Howlett@...cle.com> wrote:

> > > >
> > > > VM_SEALED isn't defined in 32-bit systems, and mseal.c isn't part of
> > > > the build. This is intentional. Any 32-bit code trying to use the
> > > > sealing function or the VM_SEALED flag will immediately fail
> > > > compilation. This makes it easier to identify incorrect usage.
> > >
> > > So you are against using the #define because the VM_SYSTEM_SEAL will be
> > > defined, even though it will be VM_NONE?  This is no worse than a
> > > function that returns 0, and it aligns better with what we have today in
> > > that it can be put in the list of other flags.
> >
> > When I was reading through all of this and considering the history of
> > its development goals, it strikes me that a function is easier for the
> > future if/when this can be made a boot-time setting.
> >
>
> Reworking this change to function as a boot-time parameter, or whatever,
> would not be a significant amount of work, if/when the time comes.
> Since we don't know what the future holds, it it unnecessary to engineer
> in a potential change for a future version when the change is easy
> enough to make later and keep the code cleaner now.
>
Sure, I will put the function in mm.h for this patch. We can find a
proper place when it is time to implement the boot-time parameter
change.

The call stack for sealing system mapping is something like below:

install_special_mapping (mm/mmap.c)
map_vdso (arch/x86/entry/vdso/vma.c)
load_elf_binary (fs/binfmt_elf.c)
load_misc_binary (fs/binfmt_misc.c)
bprm_execve (fs/exec.c)
do_execveat_common
__x64_sys_execve
do_syscall_64

IMO, there's a clear divide between the API implementer and the API user.
mm and mm.h are the providers, offering the core mm functionality
through APIs/data structures like install_special_mapping().

The exe layer (bprm_execve, map_vdso, etc)  is the consumer of the
install_special_mapping.
The logic related to checking if sealing system mapping is enabled
belongs to the exe layer.

>
> > If mm maintainers prefer a #define for now, that's fine of course. The
> > value of VM_SYSTEM_SEAL can be VM_NONE on 32-bit.
>
> Thanks.  I think having a flag with VM_NONE on 32-bit is just as sane as
> a "flags |= system_seal()" call that unconditionally returns 0 on
> 32-bit.
>
Consider the case below in src/third_party/kernel/v6.6/fs/proc/task_mmu.c,

#ifdef CONFIG_64BIT
[ilog2(VM_SEALED)] = "sl",
#endif

If #ifdef CONFIG_64BIT is missing, it won't be detected during compile time.

Setting VM_SEALED to VM_NONE could simplify the coding in some cases
(get/set case), but it might make other cases error prone.

I would prefer to not have VM_SEALED for 32 bit.


-Jeff