linux-hardening - Re: [PATCH v3 1/5] mm/mseal: always define VM

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] [day] [month] [year] [list]

Message-ID: <5bc2be6e-45a8-461a-938e-40929ef4dd4e@lucifer.local>
Date: Thu, 24 Jul 2025 19:44:30 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Jeff Xu <jeffxu@...omium.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        David Hildenbrand <david@...hat.com>, Vlastimil Babka <vbabka@...e.cz>,
        Jann Horn <jannh@...gle.com>, Pedro Falcato <pfalcato@...e.de>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Kees Cook <kees@...nel.org>, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v3 1/5] mm/mseal: always define VM_SEALED

On Thu, Jul 24, 2025 at 11:34:31AM -0700, Jeff Xu wrote:
> Hi Lorenzo,
>
> On Wed, Jul 16, 2025 at 10:38 AM Lorenzo Stoakes
> <lorenzo.stoakes@...cle.com> wrote:
> >
> > There is no reason to treat VM_SEALED in a special way, in each other case
> > in which a VMA flag is unavailable due to configuration, we simply assign
> > that flag to VM_NONE, so make VM_SEALED consistent with all other VMA
> > flags in this respect.
> >
> Originally, I wanted to avoid using VM_NONE for VM_SEALED to catch
> compiler errors in 32-bit builds. This would serve as a safeguard
> against inadvertently using VM_SEALED in code paths shared between
> 32-bit and 64-bit architectures.

I understand why you did it, and I fundamentally disagree.

This would make this the only VMA flag in the entire kernel having this
special treatment for something with very little implementation code, it
simply makes no sense.

The commit message makes this clear.

>
> Take an example of show_smap_vma_flags [1] :
>
> #ifdef CONFIG_64BIT
> [ilog2(VM_SEALED)] = "sl",
> #endif
>
> If a developer forgets to add #ifdef CONFIG_64BIT, the build will fail

This is a really silly thing to worry about.

> for 32-bit systems. With VM_SEALED redefined as VM_NONE, the problem
> will go unnoticed.

What problem exactly?

>
> This coding practice is more defensive and helps catch errors early

It's silly.

> on. It seems that you're working on introducing VM_SEALED for 32-bit
> systems. If that happens, we won't need this safeguard anymore. But
> until then, is it OK to keep it in place?  That said, if VM_SEALED
> support for 32-bit is coming really soon, we can merge this change,
> however, perhaps you could update the description to explain why we're
> removing this safeguard, i.e. due to 32-bit support will soon be in
> place.

No I won't, because that's not why I did it.