linux-kernel - Re: [PATCH] mm: perform all memfd seal checks in a single place

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: <7dee6c5d-480b-4c24-b98e-6fa47dbd8a23@lucifer.local>
Date: Mon, 9 Dec 2024 11:13:14 +0000
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Muchun Song <muchun.song@...ux.dev>, Vlastimil Babka <vbabka@...e.cz>,
        Jann Horn <jannh@...gle.com>, Hugh Dickins <hughd@...gle.com>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Isaac Manjarres <isaacmanjarres@...gle.com>,
        Kalesh Singh <kaleshsingh@...gle.com>
Subject: Re: [PATCH] mm: perform all memfd seal checks in a single place

On Fri, Dec 06, 2024 at 09:28:46PM +0000, Lorenzo Stoakes wrote:
> We no longer actually need to perform these checks in the f_op->mmap() hook
> any longer.
>
> We already moved the operation which clears VM_MAYWRITE on a read-only
> mapping of a write-sealed memfd in order to work around the restrictions
> imposed by commit 5de195060b2e ("mm: resolve faulty mmap_region() error
> path behaviour").
>
> There is no reason for us not to simply go ahead and additionally check to
> see if any pre-existing seals are in place here rather than defer this to
> the f_op->mmap() hook.
>
> By doing this we remove more logic from shmem_mmap() which doesn't belong
> there, as well as doing the same for hugetlbfs_file_mmap(). We also remove
> dubious shared logic in mm.h which simply does not belong there either.
>
> It makes sense to do these checks at the earliest opportunity, we know
> these are shmem (or hugetlbfs) mappings whose relevant VMA flags will not
> change from the invoking do_mmap() so there is simply no need to wait.
>
> This also means the implementation of further memfd seal flags can be done
> within mm/memfd.c and also have the opportunity to modify VMA flags as
> necessary early in the mapping logic.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>

Hi Andrew,

I made a rather silly typo in this patch which resulted in me failing to
mark a static inline as such and therefore, the build bots got very cross
with me :)

Apologies to all for the noise!

Could you apply the enclosed fix-patch to resolve this?

Thanks, Lorenzo

----8<----
>From 6cfef80e2ea5154302ba9b1925acd8e77ea6cd18 Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Date: Mon, 9 Dec 2024 11:04:08 +0000
Subject: [PATCH] mm: fix typos in !memfd inline stub

I typo'd the declaration of memfd_check_seals_mmap() in the case where
CONFIG_MEMFD_CREATE is not defined, resulting in build failures.

Fix this, and correct the misspelling of vm_flags which should be
vm_flags_ptr at the same time.

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
---
 include/linux/memfd.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/memfd.h b/include/linux/memfd.h
index d53408b0bd31..246daadbfde8 100644
--- a/include/linux/memfd.h
+++ b/include/linux/memfd.h
@@ -24,7 +24,8 @@ static inline struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx)
 {
 	return ERR_PTR(-EINVAL);
 }
-int memfd_check_seals_mmap(struct file *file, unsigned long *vm_flags)
+static inline int memfd_check_seals_mmap(struct file *file,
+					 unsigned long *vm_flags_ptr)
 {
 	return 0;
 }
--
2.47.1