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