| Message ID | 20240328045535.194800-3-rmclure@linux.ibm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Support page table check PowerPC | expand |
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: c0000000002951a0 LR: c0000000002951bc CTR: 0000000000000000 REGS: c0000000032e7440 TRAP: 0700 Not tainted (6.8.0-13732-gc5347beead0b-dirty) MSR: 0000000080029002 <CE,EE,ME> CR: 24044248 XER: 00000000 IRQMASK: 0 GPR00: c000000000029d90 c0000000032e76e0 c000000000d44000 c000000003017e18 GPR04: 00000000ffb11000 c000000007f16888 0000000fc324123d 0000000000000000 GPR08: 0000000000000000 0000000000000001 c000000001184000 0000000084004248 GPR12: 00000000000000c0 c0000000011b9000 c000000007f16888 c000000007f19000 GPR16: 0000000000001000 00003ffffffff000 0000000000000000 0000000000000000 GPR20: 0000400000000000 0000000000000000 0000000000000001 ffffc000ffb12000 GPR24: c000000007f19000 c000000006008000 c000000006008000 0000000000000030 GPR28: 0000000000000001 c00000000118afe8 c000000003017e18 0000000000000001 NIP [c0000000002951a0] __page_table_check_ptes_set+0x210/0x2ac LR [c0000000002951bc] __page_table_check_ptes_set+0x22c/0x2ac Call Trace: [c0000000032e76e0] [c0000000032e7790] 0xc0000000032e7790 (unreliable) [c0000000032e7730] [c000000000029d90] set_ptes+0x178/0x210 [c0000000032e7790] [c00000000024c72c] move_page_tables+0x298/0x750 [c0000000032e7870] [c0000000002a944c] shift_arg_pages+0x120/0x254 [c0000000032e79a0] [c0000000002a9f94] setup_arg_pages+0x244/0x418 [c0000000032e7b30] [c000000000331610] load_elf_binary+0x584/0x17d4 [c0000000032e7c30] [c0000000002aa3e8] bprm_execve+0x280/0x704 [c0000000032e7d00] [c0000000002ac158] kernel_execve+0x16c/0x214 [c0000000032e7d50] [c0000000000011c8] run_init_process+0x100/0x168 [c0000000032e7de0] [c00000000000214c] kernel_init+0x84/0x1f8 [c0000000032e7e50] [c000000000000594] ret_from_kernel_user_thread+0x14/0x1c --- interrupt: 0 at 0x0 Code: 81230004 7d2907b4 0b090000 7c0004ac 7d201828 31290001 7d20192d 40c2fff4 7c0004ac 2c090002 39200000 7d29e01e <0b090000> e93d0000 37ffffff 7fde4a14 ---[ end trace 0000000000000000 ]--- note: init[1] exited with irqs disabled Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000005 Rebooting in 180 seconds..
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 0x500000 CMPC885 [ 7.406481] NIP: c0183278 LR: c018316c CTR: 00000001 [ 7.411541] REGS: c902bb20 TRAP: 0700 Not tainted (6.8.0-s3k-dev-13737-g8d9e247585fb) [ 7.419657] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 35055355 XER: 80007100 [ 7.426550] [ 7.426550] GPR00: c018316c c902bbe0 c2118000 c7f7a0d8 7fab8000 c23b5ae0 c902bc20 00000002 [ 7.426550] GPR08: c11a0000 c7f7a0d8 c11143e0 00000000 95003355 00000000 c0004a38 c23a0a00 [ 7.426550] GPR16: 00004000 7fffc000 80000000 c23a0a00 00000001 7fab8000 ffabc000 80000000 [ 7.426550] GPR24: 7fffc000 c33be1c0 00004000 c902bc20 7fab8000 00000001 c7fb0360 00000000 [ 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 2c070000 4080ff94 0fe00000 0fe00000 0fe00000 2c1f0000 4082ff80 <0fe00000> 0fe00000 392affff 4bfffef8 [ 7.556068] ---[ end trace 0000000000000000 ]--- [ 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=0x00000005 [ 8.549602] Rebooting in 180 seconds..
* Rohan McLure <rmclure@linux.ibm.com> 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
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 <linux/page_table_check.h> + 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
On Thu, 2024-03-28 at 10:28 +0100, Ingo Molnar wrote: > > * Rohan McLure <rmclure@linux.ibm.com> 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
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-rmclure@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-rmclure@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-rmclure@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-rmclure@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-rmclure@linux.ibm.com/ v5: Link: https://lore.kernel.org/linuxppc-dev/20221118002146.25979-1-rmclure@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 +++--- arch/x86/include/asm/pgtable.h | 20 +++--- include/linux/page_table_check.h | 67 ++++++++++++-------- include/linux/pgtable.h | 8 +-- mm/page_table_check.c | 39 +++++++----- 17 files changed, 220 insertions(+), 98 deletions(-)