linux-kernel - Re: [RFC PATCH 1/1] mm/vmalloc: Modify permission reset procedure to avoid invalid access

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: <7b9ebf0b787222fc4e83382066a6f4e918881cf9.camel@intel.com>
Date: Tue, 18 Jun 2024 16:08:01 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "ycliang@...estech.com" <ycliang@...estech.com>
CC: "lstoakes@...il.com" <lstoakes@...il.com>, "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>, "linux-mm@...ck.org" <linux-mm@...ck.org>,
	"patrick@...estech.com" <patrick@...estech.com>, "hch@...radead.org"
	<hch@...radead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
	"urezki@...il.com" <urezki@...il.com>
Subject: Re: [RFC PATCH 1/1] mm/vmalloc: Modify permission reset procedure to
 avoid invalid access

On Tue, 2024-06-18 at 20:08 +0800, Leo Liang wrote:
> What we are seeing at first is that running LTP bpf_prog03 test fails randomly
> on RV32 SMP QEMU with kernel 6.1 and the failed cause is a load page fault.
> 
> After a bit of inspection, we found that the faulting page is a part of
> kernel's page table and the valid bit of that page's PTE is cleared due to this reset procedure.
> 
> The scenario of this fault is suspected to be the following:
> 1. Running bpf_prog03: Creates kernel pages with elevated 'X' permission so that bpf program can
> be executed.
> 2. Finishing bpf_prog03: vfree code path to reset permission to default: 
>         a. Set the pages to invalid first
>         b. Unmap the pages and flush TLB
>         c. Reset them to default permission
> 3. Other core forkes new processes: sync_kernel_mappings copies the kernel page table.
> 
> If the 3rd step happens during 2a, then we get a kernel mapping with invalid PTE permission,
> Therefore, if the invalid page is accessed, we'd get a page fault exception and the kernel panics.

So some other non-BPF related access takes the #PF I guess? How is this not a generic problem with
kernel memory permissions? There are other set_direct_map() callers.

Perhaps 32 bit riscv should not support the set_direct_map() functions if the implementation is
problematic like this. As in, something like:
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index be09c8836d56..125ba87d3c9d 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -34,7 +34,7 @@ config RISCV
        select ARCH_HAS_PMEM_API
        select ARCH_HAS_PREPARE_SYNC_CORE_CMD
        select ARCH_HAS_PTE_SPECIAL
-       select ARCH_HAS_SET_DIRECT_MAP if MMU
+       select ARCH_HAS_SET_DIRECT_MAP if MMU && 64BIT
        select ARCH_HAS_SET_MEMORY if MMU
        select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
        select ARCH_HAS_STRICT_MODULE_RWX if MMU && !XIP_KERNEL


> 
> But despite all of the above conjecture,
> we still are wondering if setting the mappings to be invalid first is necessary.
> IMHO, "set to invalid --> unmap & flush TLB --> set to default" is identical to "set to default --
> > unmap & flush TLB".
> Could we not just reset them to default first and then flush TLB & free memory?

It is trying to reset the permissions from ROX to RW without leaving a transient writable alias
while there remains an executable one, and doing it all with a single flush.

On x86 and arm, the JIT pages will have a direct map alias that is read only and a module address is
RO+X. When cleaning up, the NP PTEs can prevent any writable TLBs from being created before the
executable ones are flushed.

See here for the original discussion:
https://lore.kernel.org/lkml/20181128000754.18056-1-rick.p.edgecombe@intel.com/