FAILED: patch "[PATCH] secretmem: disable memfd_secret() if arch cannot set direct" failed to apply to 5.15-stable tree

PATCH In review [email protected] 2 participants 5 messages Oct 14, 2024 – Oct 18, 2024

Subsystems (3)

Stable branch Memory management Memory management - secretmem

lore

Share & tools

lore

[email protected]

author maintainer

2024-10-14 11:57 UTC #0 lore raw

The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.15.y
git checkout FETCH_HEAD
git cherry-pick -x 532b53cebe58f34ce1c0f34d866f5c0e335c53c6
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2024101412-prowling-snowflake-9fe0@gregkh' --subject-prefix 'PATCH 5.15.y' HEAD^..

Possible dependencies:

532b53cebe58 ("secretmem: disable memfd_secret() if arch cannot set direct map") f7c5b1aab5ef ("mm/secretmem: remove reduntant return value")

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 532b53cebe58f34ce1c0f34d866f5c0e335c53c6 Mon Sep 17 00:00:00 2001
From: Patrick Roy <[email protected]>
Date: Tue, 1 Oct 2024 09:00:41 +0100
Subject: [PATCH] secretmem: disable memfd_secret() if arch cannot set direct
map

Return -ENOSYS from memfd_secret() syscall if !can_set_direct_map(). This is the case for example on some arm64 configurations, where marking 4k PTEs in the direct map not present can only be done if the direct map is set up at 4k granularity in the first place (as ARM's break-before-make semantics do not easily allow breaking apart large/gigantic pages).

More precisely, on arm64 systems with !can_set_direct_map(), set_direct_map_invalid_noflush() is a no-op, however it returns success
(0) instead of an error. This means that memfd_secret will seemingly "work" (e.g. syscall succeeds, you can mmap the fd and fault in pages), but it does not actually achieve its goal of removing its memory from the direct map.

Note that with this patch, memfd_secret() will start erroring on systems where can_set_direct_map() returns false (arm64 with CONFIG_RODATA_FULL_DEFAULT_ENABLED=n, CONFIG_DEBUG_PAGEALLOC=n and CONFIG_KFENCE=n), but that still seems better than the current silent failure. Since CONFIG_RODATA_FULL_DEFAULT_ENABLED defaults to 'y', most arm64 systems actually have a working memfd_secret() and aren't be affected.

From going through the iterations of the original memfd_secret patch series, it seems that disabling the syscall in these scenarios was the intended behavior [1] (preferred over having set_direct_map_invalid_noflush return an error as that would result in SIGBUSes at page-fault time), however the check for it got dropped between v16 [2] and v17 [3], when secretmem moved away from CMA allocations.

[1]: https://lore.kernel.org/lkml/[email protected]/
[2]: https://lore.kernel.org/lkml/[email protected]/#t
[3]: https://lore.kernel.org/lkml/[email protected]/

Link https://lkml.kernel.org/r/[email protected] open here

Fixes 1507f51255c9 ("mm: introduce memfd_secret system call to create "secret" memory areas")

Signed-off-by Patrick Roy <[email protected]>

Reviewed-by Mike Rapoport (Microsoft) <[email protected]>

Cc Alexander Graf <[email protected]>

Cc David Hildenbrand <[email protected]>

Cc James Gowans <[email protected]>

Cc <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

mm/secretmem.c +2 -2

index 3afb5ad701e1..399552814fd0 100644
@@ -238,7 +238,7 @@ SYSCALL_DEFINE1(memfd_secret, unsigned int, flags)
 	/* make sure local flags do not confict with global fcntl.h */
 	BUILD_BUG_ON(SECRETMEM_FLAGS_MASK & O_CLOEXEC);
 
-	if (!secretmem_enable)
+	if (!secretmem_enable || !can_set_direct_map())
 		return -ENOSYS;
 
 	if (flags & ~(SECRETMEM_FLAGS_MASK | O_CLOEXEC))

@@ -280,7 +280,7 @@ static struct file_system_type secretmem_fs = {
 
 static int __init secretmem_init(void)
 {
-	if (!secretmem_enable)
+	if (!secretmem_enable || !can_set_direct_map())
 		return 0;
 
 	secretmem_mnt = kern_mount(&secretmem_fs);

Mike Rapoport

maintainer

2024-10-14 15:21 UTC #1 lore raw

PATCH 1/1 secretmem: disable memfd_secret() if arch cannot set direct map

From: Patrick Roy <[email protected]>

Quoted 1 line

From going through the iterations of the original memfd_secret patch

series, it seems that disabling the syscall in these scenarios was the intended behavior [1] (preferred over having set_direct_map_invalid_noflush return an error as that would result in SIGBUSes at page-fault time), however the check for it got dropped between v16 [2] and v17 [3], when secretmem moved away from CMA allocations.

[1]: https://lore.kernel.org/lkml/[email protected]/
[2]: https://lore.kernel.org/lkml/[email protected]/#t
[3]: https://lore.kernel.org/lkml/[email protected]/

Fixes 1507f51255c9 ("mm: introduce memfd_secret system call to create "secret" memory areas")

Signed-off-by Patrick Roy <[email protected]>

Reviewed-by Mike Rapoport (Microsoft) <[email protected]>

Signed-off-by Mike Rapoport (Microsoft) <[email protected]>

---
 mm/secretmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

mm/secretmem.c +2 -2

index d1986ce2e7c7..624663a94808 100644
@@ -234,7 +234,7 @@ SYSCALL_DEFINE1(memfd_secret, unsigned int, flags)
 	/* make sure local flags do not confict with global fcntl.h */
 	BUILD_BUG_ON(SECRETMEM_FLAGS_MASK & O_CLOEXEC);
 
-	if (!secretmem_enable)
+	if (!secretmem_enable || !can_set_direct_map())
 		return -ENOSYS;
 
 	if (flags & ~(SECRETMEM_FLAGS_MASK | O_CLOEXEC))

@@ -278,7 +278,7 @@ static int secretmem_init(void)
 {
 	int ret = 0;
 
-	if (!secretmem_enable)
+	if (!secretmem_enable || !can_set_direct_map())
 		return ret;
 
 	secretmem_mnt = kern_mount(&secretmem_fs);

Signature · 1 line

2.43.0

Greg KH

author maintainer

2024-10-14 16:12 UTC #2 lore raw

Re: [PATCH 5.15.y] secretmem: disable memfd_secret() if arch cannot set direct map

Quoted 36 lines · Mike Rapoport

From: Patrick Roy <[email protected]>
Return -ENOSYS from memfd_secret() syscall if !can_set_direct_map(). This is the case for example on some arm64 configurations, where marking 4k PTEs in the direct map not present can only be done if the direct map is set up at 4k granularity in the first place (as ARM's break-before-make semantics do not easily allow breaking apart large/gigantic pages).
More precisely, on arm64 systems with !can_set_direct_map(), set_direct_map_invalid_noflush() is a no-op, however it returns success
(0) instead of an error. This means that memfd_secret will seemingly "work" (e.g. syscall succeeds, you can mmap the fd and fault in pages), but it does not actually achieve its goal of removing its memory from the direct map.
Note that with this patch, memfd_secret() will start erroring on systems where can_set_direct_map() returns false (arm64 with CONFIG_RODATA_FULL_DEFAULT_ENABLED=n, CONFIG_DEBUG_PAGEALLOC=n and CONFIG_KFENCE=n), but that still seems better than the current silent failure. Since CONFIG_RODATA_FULL_DEFAULT_ENABLED defaults to 'y', most arm64 systems actually have a working memfd_secret() and aren't be affected.
Quoted 1 line
Quoted 1 line
From going through the iterations of the original memfd_secret patch
series, it seems that disabling the syscall in these scenarios was the intended behavior [1] (preferred over having set_direct_map_invalid_noflush return an error as that would result in SIGBUSes at page-fault time), however the check for it got dropped between v16 [2] and v17 [3], when secretmem moved away from CMA allocations.
[1]: https://lore.kernel.org/lkml/[email protected]/
[2]: https://lore.kernel.org/lkml/[email protected]/#t
[3]: https://lore.kernel.org/lkml/[email protected]/

Quoted 7 lines

Fixes 1507f51255c9 ("mm: introduce memfd_secret system call to create "secret" memory areas")
Signed-off-by Patrick Roy <[email protected]>
Reviewed-by Mike Rapoport (Microsoft) <[email protected]>
Signed-off-by Mike Rapoport (Microsoft) <[email protected]>
---
 mm/secretmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

What is the git id of this change in Linus's tree?

Mike Rapoport

maintainer

2024-10-14 16:38 UTC #3 lore raw

Re: [PATCH 5.15.y] secretmem: disable memfd_secret() if arch cannot set direct map

Quoted 45 lines · Greg KH

Quoted 36 lines · Mike Rapoport
From: Patrick Roy <[email protected]>
Return -ENOSYS from memfd_secret() syscall if !can_set_direct_map(). This is the case for example on some arm64 configurations, where marking 4k PTEs in the direct map not present can only be done if the direct map is set up at 4k granularity in the first place (as ARM's break-before-make semantics do not easily allow breaking apart large/gigantic pages).
More precisely, on arm64 systems with !can_set_direct_map(), set_direct_map_invalid_noflush() is a no-op, however it returns success
(0) instead of an error. This means that memfd_secret will seemingly "work" (e.g. syscall succeeds, you can mmap the fd and fault in pages), but it does not actually achieve its goal of removing its memory from the direct map.
Note that with this patch, memfd_secret() will start erroring on systems where can_set_direct_map() returns false (arm64 with CONFIG_RODATA_FULL_DEFAULT_ENABLED=n, CONFIG_DEBUG_PAGEALLOC=n and CONFIG_KFENCE=n), but that still seems better than the current silent failure. Since CONFIG_RODATA_FULL_DEFAULT_ENABLED defaults to 'y', most arm64 systems actually have a working memfd_secret() and aren't be affected.
Quoted 1 line
Quoted 1 line
From going through the iterations of the original memfd_secret patch
series, it seems that disabling the syscall in these scenarios was the intended behavior [1] (preferred over having set_direct_map_invalid_noflush return an error as that would result in SIGBUSes at page-fault time), however the check for it got dropped between v16 [2] and v17 [3], when secretmem moved away from CMA allocations.
[1]: https://lore.kernel.org/lkml/[email protected]/
[2]: https://lore.kernel.org/lkml/[email protected]/#t
[3]: https://lore.kernel.org/lkml/[email protected]/
Quoted 7 lines
Fixes 1507f51255c9 ("mm: introduce memfd_secret system call to create "secret" memory areas")
Signed-off-by Patrick Roy <[email protected]>
Reviewed-by Mike Rapoport (Microsoft) <[email protected]>
Signed-off-by Mike Rapoport (Microsoft) <[email protected]>
---
 mm/secretmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Quoted 1 line

What is the git id of this change in Linus's tree?

532b53cebe58f34ce1c0f34d866f5c0e335c53c6

Signature · 2 lines

Sincerely yours,
Mike.

Greg KH

author maintainer

2024-10-18 08:47 UTC #4 lore raw

Re: [PATCH 5.15.y] secretmem: disable memfd_secret() if arch cannot set direct map

Quoted 46 lines · Mike Rapoport

Quoted 45 lines · Greg KH
Quoted 43 lines · Mike Rapoport
From: Patrick Roy <[email protected]>
Return -ENOSYS from memfd_secret() syscall if !can_set_direct_map(). This is the case for example on some arm64 configurations, where marking 4k PTEs in the direct map not present can only be done if the direct map is set up at 4k granularity in the first place (as ARM's break-before-make semantics do not easily allow breaking apart large/gigantic pages).
More precisely, on arm64 systems with !can_set_direct_map(), set_direct_map_invalid_noflush() is a no-op, however it returns success
(0) instead of an error. This means that memfd_secret will seemingly "work" (e.g. syscall succeeds, you can mmap the fd and fault in pages), but it does not actually achieve its goal of removing its memory from the direct map.
Note that with this patch, memfd_secret() will start erroring on systems where can_set_direct_map() returns false (arm64 with CONFIG_RODATA_FULL_DEFAULT_ENABLED=n, CONFIG_DEBUG_PAGEALLOC=n and CONFIG_KFENCE=n), but that still seems better than the current silent failure. Since CONFIG_RODATA_FULL_DEFAULT_ENABLED defaults to 'y', most arm64 systems actually have a working memfd_secret() and aren't be affected.
Quoted 1 line
Quoted 1 line
From going through the iterations of the original memfd_secret patch
series, it seems that disabling the syscall in these scenarios was the intended behavior [1] (preferred over having set_direct_map_invalid_noflush return an error as that would result in SIGBUSes at page-fault time), however the check for it got dropped between v16 [2] and v17 [3], when secretmem moved away from CMA allocations.
[1]: https://lore.kernel.org/lkml/[email protected]/
[2]: https://lore.kernel.org/lkml/[email protected]/#t
[3]: https://lore.kernel.org/lkml/[email protected]/
Fixes 1507f51255c9 ("mm: introduce memfd_secret system call to create "secret" memory areas")
Signed-off-by Patrick Roy <[email protected]>
Reviewed-by Mike Rapoport (Microsoft) <[email protected]>
Signed-off-by Mike Rapoport (Microsoft) <[email protected]>
---
 mm/secretmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Quoted 3 lines

Quoted 1 line
What is the git id of this change in Linus's tree?
532b53cebe58f34ce1c0f34d866f5c0e335c53c6

Thanks, next time please include that in the original patch so we don't have to do this back/forth emails :)

now queued up.

greg k-h