Re: [PATCH v11 00/11] Support page table check PowerPC
On Thu, 2024-03-28 at 10:28 +0100, Ingo Molnar wrote: > > * Rohan McLure wrote: > > > Rohan McLure (11): > > Revert "mm/page_table_check: remove unused parameter in > > [__]page_table_check_pud_set" > > Revert "mm/page_table_check: remove unused parameter in > > [__]page_table_check_pmd_set" > > Revert "mm/page_table_check: remove unused parameter in > > [__]page_table_check_pud_clear" > > Revert "mm/page_table_check: remove unused parameter in > > [__]page_table_check_pmd_clear" > > Revert "mm/page_table_check: remove unused parameter in > > [__]page_table_check_pte_clear" > > Just a process request: please give these commits proper titles, they > are not really 'reverts' in the classical sense, and this title hides > what is being done in the commit. The typical use of reverts is to > revert a bad change because it broke something. Here the goal is to > reintroduce functionality. > > So please name these 5 patches accordingly, to shed light on what is > being reintroduced. You can mention it at the end of the changelog > that > it's a functional revert of commit XYZ, but that's not the primary > purpose of the commit. Thanks for your email, I'll do just that. > > Thanks, > > Ingo Cheers, Rohan
Re: [PATCH v11 00/11] Support page table check PowerPC
Le 28/03/2024 à 08:57, Christophe Leroy a écrit :
>
>
> Le 28/03/2024 à 07:52, Christophe Leroy a écrit :
>>
>>
>> Le 28/03/2024 à 05:55, Rohan McLure a écrit :
>>> Support page table check on all PowerPC platforms. This works by
>>> serialising assignments, reassignments and clears of page table
>>> entries at each level in order to ensure that anonymous mappings
>>> have at most one writable consumer, and likewise that file-backed
>>> mappings are not simultaneously also anonymous mappings.
>>>
>>> In order to support this infrastructure, a number of stubs must be
>>> defined for all powerpc platforms. Additionally, seperate set_pte_at()
>>> and set_pte_at_unchecked(), to allow for internal, uninstrumented
>>> mappings.
>>
>> I gave it a try on QEMU e500 (64 bits), and get the following Oops.
>> What do I have to look for ?
>>
>> Freeing unused kernel image (initmem) memory: 2588K
>> This architecture does not have kernel memory protection.
>> Run /init as init process
>> [ cut here ]
>> kernel BUG at mm/page_table_check.c:119!
>> Oops: Exception in kernel mode, sig: 5 [#1]
>> BE PAGE_SIZE=4K SMP NR_CPUS=32 QEMU e500
>
> Same problem on my 8xx board:
>
> [ 7.358146] Freeing unused kernel image (initmem) memory: 448K
> [ 7.363957] Run /init as init process
> [ 7.370955] [ cut here ]
> [ 7.375411] kernel BUG at mm/page_table_check.c:119!
> [ 7.380393] Oops: Exception in kernel mode, sig: 5 [#1]
> [ 7.385621] BE PAGE_SIZE=16K PREEMPT CMPC885
Both problems are fixed by following change:
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h
b/arch/powerpc/include/asm/nohash/pgtable.h
index 413d01a51e6f..5b932632a5d7 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@ -29,6 +29,8 @@ static inline pte_basic_t pte_update(struct mm_struct
*mm, unsigned long addr, p
#ifndef __ASSEMBLY__
+#include
+
extern int icache_44x_need_flush;
/*
@@ -92,7 +94,11 @@ static inline void ptep_set_wrprotect(struct
mm_struct *mm, unsigned long addr,
static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned
long addr,
pte_t *ptep)
{
- return __pte(pte_update(mm, addr, ptep, ~0UL, 0, 0));
+ pte_t old_pte = __pte(pte_update(mm, addr, ptep, ~0UL, 0, 0));
+
+ page_table_check_pte_clear(mm, addr, old_pte);
+
+ return old_pte;
}
#define __HAVE_ARCH_PTEP_GET_AND_CLEAR
Re: [PATCH v11 00/11] Support page table check PowerPC
* Rohan McLure wrote: > Rohan McLure (11): > Revert "mm/page_table_check: remove unused parameter in > [__]page_table_check_pud_set" > Revert "mm/page_table_check: remove unused parameter in > [__]page_table_check_pmd_set" > Revert "mm/page_table_check: remove unused parameter in > [__]page_table_check_pud_clear" > Revert "mm/page_table_check: remove unused parameter in > [__]page_table_check_pmd_clear" > Revert "mm/page_table_check: remove unused parameter in > [__]page_table_check_pte_clear" Just a process request: please give these commits proper titles, they are not really 'reverts' in the classical sense, and this title hides what is being done in the commit. The typical use of reverts is to revert a bad change because it broke something. Here the goal is to reintroduce functionality. So please name these 5 patches accordingly, to shed light on what is being reintroduced. You can mention it at the end of the changelog that it's a functional revert of commit XYZ, but that's not the primary purpose of the commit. Thanks, Ingo
Re: [PATCH v11 00/11] Support page table check PowerPC
Le 28/03/2024 à 07:52, Christophe Leroy a écrit : > > > Le 28/03/2024 à 05:55, Rohan McLure a écrit : >> Support page table check on all PowerPC platforms. This works by >> serialising assignments, reassignments and clears of page table >> entries at each level in order to ensure that anonymous mappings >> have at most one writable consumer, and likewise that file-backed >> mappings are not simultaneously also anonymous mappings. >> >> In order to support this infrastructure, a number of stubs must be >> defined for all powerpc platforms. Additionally, seperate set_pte_at() >> and set_pte_at_unchecked(), to allow for internal, uninstrumented >> mappings. > > I gave it a try on QEMU e500 (64 bits), and get the following Oops. What > do I have to look for ? > > Freeing unused kernel image (initmem) memory: 2588K > This architecture does not have kernel memory protection. > Run /init as init process > [ cut here ] > kernel BUG at mm/page_table_check.c:119! > Oops: Exception in kernel mode, sig: 5 [#1] > BE PAGE_SIZE=4K SMP NR_CPUS=32 QEMU e500 Same problem on my 8xx board: [7.358146] Freeing unused kernel image (initmem) memory: 448K [7.363957] Run /init as init process [7.370955] [ cut here ] [7.375411] kernel BUG at mm/page_table_check.c:119! [7.380393] Oops: Exception in kernel mode, sig: 5 [#1] [7.385621] BE PAGE_SIZE=16K PREEMPT CMPC885 [7.393483] CPU: 0 PID: 1 Comm: init Not tainted 6.8.0-s3k-dev-13737-g8d9e247585fb #787 [7.401505] Hardware name: MIAE 8xx 0x50 CMPC885 [7.406481] NIP: c0183278 LR: c018316c CTR: 0001 [7.411541] REGS: c902bb20 TRAP: 0700 Not tainted (6.8.0-s3k-dev-13737-g8d9e247585fb) [7.419657] MSR: 00029032 CR: 35055355 XER: 80007100 [7.426550] [7.426550] GPR00: c018316c c902bbe0 c2118000 c7f7a0d8 7fab8000 c23b5ae0 c902bc20 0002 [7.426550] GPR08: c11a c7f7a0d8 c11143e0 95003355 c0004a38 c23a0a00 [7.426550] GPR16: 4000 7fffc000 8000 c23a0a00 0001 7fab8000 ffabc000 8000 [7.426550] GPR24: 7fffc000 c33be1c0 4000 c902bc20 7fab8000 0001 c7fb0360 [7.463291] NIP [c0183278] __page_table_check_ptes_set+0x1c8/0x210 [7.469491] LR [c018316c] __page_table_check_ptes_set+0xbc/0x210 [7.475514] Call Trace: [7.477957] [c902bbe0] [c018316c] __page_table_check_ptes_set+0xbc/0x210 (unreliable) [7.485809] [c902bc00] [c0012474] set_ptes+0x148/0x16c [7.490958] [c902bc50] [c0158a3c] move_page_tables+0x228/0x578 [7.496806] [c902bcf0] [c0192f38] shift_arg_pages+0xf0/0x1d4 [7.502479] [c902bd90] [c0193b40] setup_arg_pages+0x1c8/0x36c [7.508238] [c902be40] [c01f51a0] load_elf_binary+0x3c0/0x1218 [7.514086] [c902beb0] [c01934b0] bprm_execve+0x21c/0x4a4 [7.519497] [c902bf00] [c019516c] kernel_execve+0x13c/0x200 [7.525082] [c902bf20] [c0004aa8] kernel_init+0x70/0x1b0 [7.530406] [c902bf30] [c00111e4] ret_from_kernel_user_thread+0x10/0x18 [7.537038] --- interrupt: 0 at 0x0 [7.540534] Code: 39290004 7ce04828 30e70001 7ce0492d 40a2fff4 2c07 4080ff94 0fe0 0fe0 0fe0 2c1f 4082ff80 <0fe0> 0fe0 392a 4bfffef8 [7.556068] ---[ end trace ]--- [7.560692] [8.531997] note: init[1] exited with irqs disabled [8.536891] note: init[1] exited with preempt_count 1 [8.542032] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0005 [8.549602] Rebooting in 180 seconds..
Re: [PATCH v11 00/11] Support page table check PowerPC
Le 28/03/2024 à 05:55, Rohan McLure a écrit : > Support page table check on all PowerPC platforms. This works by > serialising assignments, reassignments and clears of page table > entries at each level in order to ensure that anonymous mappings > have at most one writable consumer, and likewise that file-backed > mappings are not simultaneously also anonymous mappings. > > In order to support this infrastructure, a number of stubs must be > defined for all powerpc platforms. Additionally, seperate set_pte_at() > and set_pte_at_unchecked(), to allow for internal, uninstrumented mappings. I gave it a try on QEMU e500 (64 bits), and get the following Oops. What do I have to look for ? Freeing unused kernel image (initmem) memory: 2588K This architecture does not have kernel memory protection. Run /init as init process [ cut here ] kernel BUG at mm/page_table_check.c:119! Oops: Exception in kernel mode, sig: 5 [#1] BE PAGE_SIZE=4K SMP NR_CPUS=32 QEMU e500 Modules linked in: CPU: 0 PID: 1 Comm: init Not tainted 6.8.0-13732-gc5347beead0b-dirty #784 Hardware name: QEMU ppce500 e5500 0x80240020 QEMU e500 NIP: c02951a0 LR: c02951bc CTR: REGS: c32e7440 TRAP: 0700 Not tainted (6.8.0-13732-gc5347beead0b-dirty) MSR: 80029002 CR: 24044248 XER: IRQMASK: 0 GPR00: c0029d90 c32e76e0 c0d44000 c3017e18 GPR04: ffb11000 c7f16888 000fc324123d GPR08: 0001 c1184000 84004248 GPR12: 00c0 c11b9000 c7f16888 c7f19000 GPR16: 1000 3000 GPR20: 4000 0001 c000ffb12000 GPR24: c7f19000 c6008000 c6008000 0030 GPR28: 0001 c118afe8 c3017e18 0001 NIP [c02951a0] __page_table_check_ptes_set+0x210/0x2ac LR [c02951bc] __page_table_check_ptes_set+0x22c/0x2ac Call Trace: [c32e76e0] [c32e7790] 0xc32e7790 (unreliable) [c32e7730] [c0029d90] set_ptes+0x178/0x210 [c32e7790] [c024c72c] move_page_tables+0x298/0x750 [c32e7870] [c02a944c] shift_arg_pages+0x120/0x254 [c32e79a0] [c02a9f94] setup_arg_pages+0x244/0x418 [c32e7b30] [c0331610] load_elf_binary+0x584/0x17d4 [c32e7c30] [c02aa3e8] bprm_execve+0x280/0x704 [c32e7d00] [c02ac158] kernel_execve+0x16c/0x214 [c32e7d50] [c00011c8] run_init_process+0x100/0x168 [c32e7de0] [c000214c] kernel_init+0x84/0x1f8 [c32e7e50] [c594] ret_from_kernel_user_thread+0x14/0x1c --- interrupt: 0 at 0x0 Code: 81230004 7d2907b4 0b09 7c0004ac 7d201828 31290001 7d20192d 40c2fff4 7c0004ac 2c090002 3920 7d29e01e <0b09> e93d 37ff 7fde4a14 ---[ end trace ]--- note: init[1] exited with irqs disabled Kernel panic - not syncing: Attempted to kill init! exitcode=0x0005 Rebooting in 180 seconds..
[PATCH v11 00/11] Support page table check PowerPC
Support page table check on all PowerPC platforms. This works by
serialising assignments, reassignments and clears of page table
entries at each level in order to ensure that anonymous mappings
have at most one writable consumer, and likewise that file-backed
mappings are not simultaneously also anonymous mappings.
In order to support this infrastructure, a number of stubs must be
defined for all powerpc platforms. Additionally, seperate set_pte_at()
and set_pte_at_unchecked(), to allow for internal, uninstrumented mappings.
v11:
* The pud_pfn() stub, which previously had no legitimate users on any
powerpc platform, now has users in Book3s64 with transparent pages.
Include a stub of the same name for each platform that does not
define their own.
* Drop patch that standardised use of p*d_leaf(), as already included
upstream in v6.9.
* Provide fallback definitions of p{m,u}d_user_accessible_page() that
do not reference p*d_leaf(), p*d_pte(), as they are defined after
powerpc/mm headers by linux/mm headers.
* Ensure that set_pte_at_unchecked() has the same checks as
set_pte_at().
v10:
* Revert patches that removed address and mm parameters from page table
check routines, including consuming code from arm64, x86_64 and
riscv.
* Implement *_user_accessible_page() routines in terms of pte_user()
where available (64-bit, book3s) but otherwise by checking the
address (on platforms where the pte does not imply whether the
mapping is for user or kernel)
* Internal set_pte_at() calls replaced with set_pte_at_unchecked(), which
is identical, but prevents double instrumentation.
Link:
https://lore.kernel.org/linuxppc-dev/20240313042118.230397-9-rmcl...@linux.ibm.com/T/
v9:
* Adapt to using the set_ptes() API, using __set_pte_at() where we need
must avoid instrumentation.
* Use the logic of *_access_permitted() for implementing
*_user_accessible_page(), which are required routines for page table
check.
* Even though we no longer need p{m,u,4}d_leaf(), still default
implement these to assist in refactoring out extant
p{m,u,4}_is_leaf().
* Add p{m,u}_pte() stubs where asm-generic does not provide them, as
page table check wants all *user_accessible_page() variants, and we
would like to default implement the variants in terms of
pte_user_accessible_page().
* Avoid the ugly pmdp_collapse_flush() macro nonsense! Just instrument
its constituent calls instead for radix and hash.
Link:
https://lore.kernel.org/linuxppc-dev/20231130025404.37179-2-rmcl...@linux.ibm.com/
v8:
* Fix linux/page_table_check.h include in asm/pgtable.h breaking
32-bit.
Link:
https://lore.kernel.org/linuxppc-dev/20230215231153.2147454-1-rmcl...@linux.ibm.com/
v7:
* Remove use of extern in set_pte prototypes
* Clean up pmdp_collapse_flush macro
* Replace set_pte_at with static inline function
* Fix commit message for patch 7
Link:
https://lore.kernel.org/linuxppc-dev/20230215020155.1969194-1-rmcl...@linux.ibm.com/
v6:
* Support huge pages and p{m,u}d accounting.
* Remove instrumentation from set_pte from kernel internal pages.
* 64s: Implement pmdp_collapse_flush in terms of __pmdp_collapse_flush
as access to the mm_struct * is required.
Link:
https://lore.kernel.org/linuxppc-dev/20230214015939.1853438-1-rmcl...@linux.ibm.com/
v5:
Link:
https://lore.kernel.org/linuxppc-dev/20221118002146.25979-1-rmcl...@linux.ibm.com/
Rohan McLure (11):
Revert "mm/page_table_check: remove unused parameter in
[__]page_table_check_pud_set"
Revert "mm/page_table_check: remove unused parameter in
[__]page_table_check_pmd_set"
mm: Provide addr parameter to page_table_check_pte_set()
Revert "mm/page_table_check: remove unused parameter in
[__]page_table_check_pud_clear"
Revert "mm/page_table_check: remove unused parameter in
[__]page_table_check_pmd_clear"
Revert "mm/page_table_check: remove unused parameter in
[__]page_table_check_pte_clear"
mm: Provide address parameter to p{te,md,ud}_user_accessible_page()
powerpc: mm: Add pud_pfn() stub
poweprc: mm: Implement *_user_accessible_page() for ptes
powerpc: mm: Use set_pte_at_unchecked() for early-boot / internal
usages
powerpc: mm: Support page table check
arch/arm64/include/asm/pgtable.h | 18 +++---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/book3s/32/pgtable.h | 12 +++-
arch/powerpc/include/asm/book3s/64/pgtable.h | 62 +++---
arch/powerpc/include/asm/nohash/pgtable.h| 5 ++
arch/powerpc/include/asm/pgtable.h | 18 ++
arch/powerpc/mm/book3s64/hash_pgtable.c | 6 +-
arch/powerpc/mm/book3s64/pgtable.c | 17 +++--
arch/powerpc/mm/book3s64/radix_pgtable.c | 11 ++--
arch/powerpc/mm/nohash/book3e_pgtable.c | 2 +-
arch/powerpc/mm/pgtable.c| 12
arch/powerpc/mm/pgtable_32.c | 2 +-
arch/riscv/include/asm/pgtable.h | 18
