Implements the "init_mlocked_on_free" boot option. When this boot option
is enabled, any mlock'ed pages are zeroed on free. If
the pages are munlock'ed beforehand, no initialization takes place.
This boot option is meant to combat the performance hit of
"init_on_free" as reported in commit 6471384af2a6 ("mm: security:
introduce init_on_alloc=1 and init_on_free=1 boot options"). With
"init_mlocked_on_free=1" only relevant data is freed while everything
else is left untouched by the kernel. Correspondingly, this patch
introduces no performance hit for unmapping non-mlock'ed memory. The
unmapping overhead for purely mlocked memory was measured to be
approximately 13%. Realistically, most systems mlock only a fraction of
the total memory so the real-world system overhead should be close to
zero.
Optimally, userspace programs clear any key material or other
confidential memory before exit and munlock the according memory
regions. If a program crashes, userspace key managers fail to do this
job. Accordingly, no munlock operations are performed so the data is
caught and zeroed by the kernel. Should the program not crash, all
memory will ideally be munlocked so no overhead is caused.
CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON can be set to enable
"init_mlocked_on_free" by default.
Signed-off-by: York Jasper Niebuhr <[email protected]>
---
.../admin-guide/kernel-parameters.txt | 6 +++
include/linux/mm.h | 9 +++-
mm/internal.h | 1 +
mm/memory.c | 6 +++
mm/mm_init.c | 43 ++++++++++++++++---
mm/page_alloc.c | 2 +-
security/Kconfig.hardening | 15 +++++++
7 files changed, 73 insertions(+), 9 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index bb884c14b2f6..34bdbf29aaf9 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2148,6 +2148,12 @@
Format: 0 | 1
Default set by CONFIG_INIT_ON_FREE_DEFAULT_ON.
+ init_mlocked_on_free= [MM] Fill freed userspace memory with zeroes if
+ it was mlock'ed and not explicitly munlock'ed
+ afterwards.
+ Format: 0 | 1
+ Default set by CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON
+
init_pkru= [X86] Specify the default memory protection keys rights
register contents for all processes. 0x55555554 by
default (disallow access to all but pkey 0). Can
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 8eccaabe44ee..a216489aef2b 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3751,7 +3751,14 @@ DECLARE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_FREE_DEFAULT_ON, init_on_free);
static inline bool want_init_on_free(void)
{
return static_branch_maybe(CONFIG_INIT_ON_FREE_DEFAULT_ON,
- &init_on_free);
+ &init_on_free);
+}
+
+DECLARE_STATIC_KEY_MAYBE(CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON, init_mlocked_on_free);
+static inline bool want_init_mlocked_on_free(void)
+{
+ return static_branch_maybe(CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON,
+ &init_mlocked_on_free);
}
extern bool _debug_pagealloc_enabled_early;
diff --git a/mm/internal.h b/mm/internal.h
index 8e11f7b2da21..8ec85b5e62e6 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -506,6 +506,7 @@ extern void __putback_isolated_page(struct page *page, unsigned int order,
extern void memblock_free_pages(struct page *page, unsigned long pfn,
unsigned int order);
extern void __free_pages_core(struct page *page, unsigned int order);
+extern void kernel_init_pages(struct page *page, int numpages);
/*
* This will have no effect, other than possibly generating a warning, if the
diff --git a/mm/memory.c b/mm/memory.c
index 36191a9c799c..706361b9d1af 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1506,6 +1506,12 @@ static __always_inline void zap_present_folio_ptes(struct mmu_gather *tlb,
if (unlikely(page_mapcount(page) < 0))
print_bad_pte(vma, addr, ptent, page);
}
+
+ if (want_init_mlocked_on_free() && folio_test_mlocked(folio) &&
+ !delay_rmap && folio_test_anon(folio)) {
+ kernel_init_pages(page, folio_nr_pages(folio));
+ }
+
if (unlikely(__tlb_remove_folio_pages(tlb, page, nr, delay_rmap))) {
*force_flush = true;
*force_break = true;
diff --git a/mm/mm_init.c b/mm/mm_init.c
index e3450f12d79a..bede28e8d7cc 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -2515,6 +2515,9 @@ EXPORT_SYMBOL(init_on_alloc);
DEFINE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_FREE_DEFAULT_ON, init_on_free);
EXPORT_SYMBOL(init_on_free);
+DEFINE_STATIC_KEY_MAYBE(CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON, init_mlocked_on_free);
+EXPORT_SYMBOL(init_mlocked_on_free);
+
static bool _init_on_alloc_enabled_early __read_mostly
= IS_ENABLED(CONFIG_INIT_ON_ALLOC_DEFAULT_ON);
static int __init early_init_on_alloc(char *buf)
@@ -2532,6 +2535,14 @@ static int __init early_init_on_free(char *buf)
}
early_param("init_on_free", early_init_on_free);
+static bool _init_mlocked_on_free_enabled_early __read_mostly
+ = IS_ENABLED(CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON);
+static int __init early_init_mlocked_on_free(char *buf)
+{
+ return kstrtobool(buf, &_init_mlocked_on_free_enabled_early);
+}
+early_param("init_mlocked_on_free", early_init_mlocked_on_free);
+
DEFINE_STATIC_KEY_MAYBE(CONFIG_DEBUG_VM, check_pages_enabled);
/*
@@ -2559,12 +2570,21 @@ static void __init mem_debugging_and_hardening_init(void)
}
#endif
- if ((_init_on_alloc_enabled_early || _init_on_free_enabled_early) &&
+ if ((_init_on_alloc_enabled_early || _init_on_free_enabled_early ||
+ _init_mlocked_on_free_enabled_early) &&
page_poisoning_requested) {
pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
- "will take precedence over init_on_alloc and init_on_free\n");
+ "will take precedence over init_on_alloc, init_on_free "
+ "and init_mlocked_on_free\n");
_init_on_alloc_enabled_early = false;
_init_on_free_enabled_early = false;
+ _init_mlocked_on_free_enabled_early = false;
+ }
+
+ if (_init_mlocked_on_free_enabled_early && _init_on_free_enabled_early) {
+ pr_info("mem auto-init: init_on_free is on, "
+ "will take precedence over init_mlocked_on_free\n");
+ _init_mlocked_on_free_enabled_early = false;
}
if (_init_on_alloc_enabled_early) {
@@ -2581,9 +2601,17 @@ static void __init mem_debugging_and_hardening_init(void)
static_branch_disable(&init_on_free);
}
- if (IS_ENABLED(CONFIG_KMSAN) &&
- (_init_on_alloc_enabled_early || _init_on_free_enabled_early))
- pr_info("mem auto-init: please make sure init_on_alloc and init_on_free are disabled when running KMSAN\n");
+ if (_init_mlocked_on_free_enabled_early) {
+ want_check_pages = true;
+ static_branch_enable(&init_mlocked_on_free);
+ } else {
+ static_branch_disable(&init_mlocked_on_free);
+ }
+
+ if (IS_ENABLED(CONFIG_KMSAN) && (_init_on_alloc_enabled_early ||
+ _init_on_free_enabled_early || _init_mlocked_on_free_enabled_early))
+ pr_info("mem auto-init: please make sure init_on_alloc, init_on_free and "
+ "init_mlocked_on_free are disabled when running KMSAN\n");
#ifdef CONFIG_DEBUG_PAGEALLOC
if (debug_pagealloc_enabled()) {
@@ -2622,9 +2650,10 @@ static void __init report_meminit(void)
else
stack = "off";
- pr_info("mem auto-init: stack:%s, heap alloc:%s, heap free:%s\n",
+ pr_info("mem auto-init: stack:%s, heap alloc:%s, heap free:%s, mlocked free:%s\n",
stack, want_init_on_alloc(GFP_KERNEL) ? "on" : "off",
- want_init_on_free() ? "on" : "off");
+ want_init_on_free() ? "on" : "off",
+ want_init_mlocked_on_free() ? "on" : "off");
if (want_init_on_free())
pr_info("mem auto-init: clearing system memory may take some time...\n");
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5b8ab1bfb9d5..8a8ca555b9d6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1038,7 +1038,7 @@ static inline bool should_skip_kasan_poison(struct page *page)
return page_kasan_tag(page) == KASAN_TAG_KERNEL;
}
-static void kernel_init_pages(struct page *page, int numpages)
+void kernel_init_pages(struct page *page, int numpages)
{
int i;
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index 2cff851ebfd7..effbf5982be1 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -255,6 +255,21 @@ config INIT_ON_FREE_DEFAULT_ON
touching "cold" memory areas. Most cases see 3-5% impact. Some
synthetic workloads have measured as high as 8%.
+config INIT_MLOCKED_ON_FREE_DEFAULT_ON
+ bool "Enable mlocked memory zeroing on free"
+ depends on !KMSAN
+ help
+ This config has the effect of setting "init_mlocked_on_free=1"
+ on the kernel command line. If it is enabled, all mlocked process
+ memory is zeroed when freed. This restriction to mlocked memory
+ improves performance over "init_on_free" but can still be used to
+ protect confidential data like key material from content exposures
+ to other processes, as well as live forensics and cold boot attacks.
+ Any non-mlocked memory is not cleared before it is reassigned. This
+ configuration can be overwritten by setting "init_mlocked_on_free=0"
+ on the command line. The "init_on_free" boot option takes
+ precedence over "init_mlocked_on_free".
+
config CC_HAS_ZERO_CALL_USED_REGS
def_bool $(cc-option,-fzero-call-used-regs=used-gpr)
# https://github.com/ClangBuiltLinux/linux/issues/1766
base-commit: a6bd6c9333397f5a0e2667d4d82fef8c970108f2
--
2.34.1
On Fri, Mar 29, 2024 at 7:56 AM York Jasper Niebuhr
<[email protected]> wrote:
>
> Implements the "init_mlocked_on_free" boot option. When this boot option
> is enabled, any mlock'ed pages are zeroed on free. If
> the pages are munlock'ed beforehand, no initialization takes place.
> This boot option is meant to combat the performance hit of
> "init_on_free" as reported in commit 6471384af2a6 ("mm: security:
> introduce init_on_alloc=1 and init_on_free=1 boot options"). With
I understand the intent of the init_on_alloc and init_on_free options,
but what's the idea behind special-casing on mlock?
Is the idea that mlocking implies something other than "preventing
memory from being swapped out"?
> "init_mlocked_on_free=1" only relevant data is freed while everything
> else is left untouched by the kernel. Correspondingly, this patch
> introduces no performance hit for unmapping non-mlock'ed memory. The
> unmapping overhead for purely mlocked memory was measured to be
> approximately 13%. Realistically, most systems mlock only a fraction of
> the total memory so the real-world system overhead should be close to
> zero.
>
> Optimally, userspace programs clear any key material or other
> confidential memory before exit and munlock the according memory
> regions. If a program crashes, userspace key managers fail to do this
> job. Accordingly, no munlock operations are performed so the data is
> caught and zeroed by the kernel. Should the program not crash, all
> memory will ideally be munlocked so no overhead is caused.
>
> CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON can be set to enable
> "init_mlocked_on_free" by default.
>
> Signed-off-by: York Jasper Niebuhr <[email protected]>
FYI, git format-patch takes a -v parameter to specify the version of
the patch series. and scripts/checkpatch.pl should catch some of the
formatting and style issues.
I also accidentally forgot to reply all, sorry about the noise York.
Thanks,
Yuanchu Xie
On 29.03.24 15:56, York Jasper Niebuhr wrote:
> Implements the "init_mlocked_on_free" boot option. When this boot option
> is enabled, any mlock'ed pages are zeroed on free. If
> the pages are munlock'ed beforehand, no initialization takes place.
> This boot option is meant to combat the performance hit of
> "init_on_free" as reported in commit 6471384af2a6 ("mm: security:
> introduce init_on_alloc=1 and init_on_free=1 boot options"). With
> "init_mlocked_on_free=1" only relevant data is freed while everything
> else is left untouched by the kernel. Correspondingly, this patch
> introduces no performance hit for unmapping non-mlock'ed memory. The
> unmapping overhead for purely mlocked memory was measured to be
> approximately 13%. Realistically, most systems mlock only a fraction of
> the total memory so the real-world system overhead should be close to
> zero.
>
> Optimally, userspace programs clear any key material or other
> confidential memory before exit and munlock the according memory
> regions. If a program crashes, userspace key managers fail to do this
> job. Accordingly, no munlock operations are performed so the data is
> caught and zeroed by the kernel. Should the program not crash, all
> memory will ideally be munlocked so no overhead is caused.
>
> CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON can be set to enable
> "init_mlocked_on_free" by default.
>
> Signed-off-by: York Jasper Niebuhr <[email protected]>
I'm not convinced that this is the right approach.
You seem to be focused on "don't leak secrets stored in user space
somewhere else". Well, and assuming that no other users on such systems
use mlock() for a different purpose where the additional clearing will
just be overhead.
In general, I'm not a fan of any such kernel cmdline options. Really, we
want to handle memory that stores secrets always in a sane way.
Note that in the meantime, we do have secretmem for that purpose, which
primary use case -- in contrast to mlock -- is to store secrets.
I now that "teach user space to use secretmem" is not a good answer, but
further emphasizing "mlock means storing secrets" feels wrong.
Also note that your patch won't handle all cases: mlocked folios can be
migrated in memory. But there is no such code that handles freeing of
the source page during migration, so you could still leak memory at
least there ...
(I was briefly thinking about a VMA option, independent of mlock, to
achieve that. But likely just using secretmem might be the better
long-term approach)
--
Cheers,
David / dhildenb
Completely agree with David's point[1]. I'm also not convinced that this is the
right approach :)
It seems like this patch won't handle all cases, as David mentioned[1] before.
folio_remove_rmap_ptes() will immediately munlock a large folio (as large folios
are not allowed to be batch-added to the LRU list) via munlock_vma_folio() when
it is fully unmapped, so this patch does not work in this case. Even worse, if
we encounter a COW mlocked folio, we would run into trouble (data corruption).
Hi Andrew, I just noticed that this patch has become part of v6.10-rc1, but it
has not been acked/reviewed yet. Is there any chance to revert it?
[1] https://lore.kernel.org/linux-mm/[email protected]/
[2] https://lore.kernel.org/linux-mm/[email protected]/
Thanks,
Lance
On 01.06.24 16:09, Lance Yang wrote:
> Completely agree with David's point[1]. I'm also not convinced that this is the
> right approach :)
>
> It seems like this patch won't handle all cases, as David mentioned[1] before.
> folio_remove_rmap_ptes() will immediately munlock a large folio (as large folios
> are not allowed to be batch-added to the LRU list) via munlock_vma_folio() when
> it is fully unmapped, so this patch does not work in this case. Even worse, if
> we encounter a COW mlocked folio, we would run into trouble (data corruption).
>
> Hi Andrew, I just noticed that this patch has become part of v6.10-rc1, but it
> has not been acked/reviewed yet. Is there any chance to revert it?
Thanks Lance, for paying attention. I think I spotted this on LWN and
thought "I don't recall that we agreed this is the right approach" but
didn't have time to follow up.
My opinion on this did not change.
--
Cheers,
David / dhildenb
On 01.06.24 16:09, Lance Yang wrote:
> Completely agree with David's point[1]. I'm also not convinced that this is the
> right approach :)
>
> It seems like this patch won't handle all cases, as David mentioned[1] before.
> folio_remove_rmap_ptes() will immediately munlock a large folio (as large folios
> are not allowed to be batch-added to the LRU list) via munlock_vma_folio() when
> it is fully unmapped, so this patch does not work in this case. Even worse, if
> we encounter a COW mlocked folio, we would run into trouble (data corruption).
Adding to what Lance said regarding COW, here is a simple reproducer:
[root@localhost ~]# cat mlock.c
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/wait.h>
int main(void)
{
size_t size = getpagesize();
volatile char *mem;
int rc;
mem = mmap(0, size, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0);
if (mem == MAP_FAILED) {
fprintf(stderr, "mmap() failed\n");
return -1;
}
rc = mlock((char *) mem, size);
if (rc) {
fprintf(stderr, "mlock() failed\n");
return -1;
}
memset((char *) mem, 1, size);
rc = fork();
if (rc < 0) {
fprintf(stderr, "fork() failed\n");
return -1;
} else if (!rc) {
return 0;
}
waitpid(rc, NULL, 0);
sleep(2);
if (mem[0] != 1) {
fprintf(stderr, "[FAIL] Memory content changed!\n");
return -1;
}
fprintf(stderr, "[PASS] Memory content did not change!\n");
return 0;
}
[root@localhost ~]# ./mlock
[FAIL] Memory content changed!
In contrast, when it's not enabled on the cmdline:
[root@localhost ~]# ./mlock
[PASS] Memory content did not change!
Further, rebooting with this enabled gives me (retried once to make sure enabling/disabling
this feature makes a difference):
[ 105.057230] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000008b
[ 105.059491] CPU: 29 PID: 1 Comm: systemd-shutdow Not tainted 6.10.0-rc1+ #16
[ 105.061545] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-2.fc40 04/01/2014
[ 105.063884] Call Trace:
[ 105.064548] <TASK>
[ 105.065088] dump_stack_lvl+0x5d/0x80
[ 105.066036] panic+0x118/0x2c8
[ 105.066837] do_exit.cold+0x18/0x18
[ 105.067759] do_group_exit+0x36/0xa0
[ 105.069177] get_signal+0x9bc/0x9e0
[ 105.070191] arch_do_signal_or_restart+0x3b/0x240
[ 105.071538] irqentry_exit_to_user_mode+0x1c2/0x230
[ 105.072913] asm_exc_page_fault+0x26/0x30
[ 105.074057] RIP: 0033:0x0
[ 105.074803] Code: Unable to access opcode bytes at 0xffffffffffffffd6.
[ 105.076647] RSP: 002b:00007fff0d9a41a8 EFLAGS: 00010206
[ 105.078090] RAX: 0000000000000011 RBX: 0000000006d8cef9 RCX: 0000000000000005
[ 105.080046] RDX: 00007fff0d9a4250 RSI: 00000000000008c7 RDI: 0000000000000001
[ 105.082139] RBP: 00007fff0d9a4310 R08: 0019fa9fcd800000 R09: 00000029e0b9a6f0
[ 105.084038] R10: 0000000000000008 R11: 0000000000000202 R12: 00007fff0d9a4250
[ 105.085852] R13: 00007fff0d9a41d0 R14: 00000000000008c7 R15: 0000000000000000
[ 105.087653] </TASK>
[ 105.090686] Kernel Offset: 0x1e000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[ 105.093359] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000008b ]---
This needs to be reverted.
@Andrew, to you want us to send an official revert patch?
--
Cheers,
David / dhildenb