| Message ID | 20210326070755.304625-1-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | powerpc/mm/book3s64: Use the correct storage key value when calling H_PROTECT | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (87d76f542a24ecfa797e9bd3bb56c0f19aabff57) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 2 warnings, 0 checks, 9 lines checked |
| snowpatch_ozlabs/needsstable | warning | Please consider tagging this patch for stable! |
On Friday, March 26, 2021 4:07:55 AM -03 Aneesh Kumar K.V wrote: > H_PROTECT expect the flag value to include > flags: AVPN, pp0, pp1, pp2, key0-key4, Noexec, CMO Option flags > > This patch updates hpte_updatepp() to fetch the storage key value from the > linux page table and use the same in H_PROTECT hcall. > > native_hpte_updatepp() is not updated because the kernel doesn't clear the > existing storage key value there. The kernel also doesn't use > hpte_updatepp() callback for updating storage keys. > > This fixes the below kernel crash observed with KUAP enabled. > > BUG: Unable to handle kernel data access on write at 0xc009fffffc440000 > Faulting instruction address: 0xc0000000000b7030 > Key fault AMR: 0xfcffffffffffffff IAMR: 0xc0000077bc498100 > Found HPTE: v = 0x40070adbb6fffc05 r = 0x1ffffffffff1194 > Oops: Kernel access of bad area, sig: 11 [#1] > LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries > ......... > CFAR: c000000000010100 DAR: c009fffffc440000 DSISR: 02200000 IRQMASK: 0 > .......... > NIP [c0000000000b7030] memset+0x68/0x104 > LR [c0000000003ef00c] pcpu_alloc+0x54c/0xb50 > Call Trace: > [c00000001c7534f0] [c0000000003ef01c] pcpu_alloc+0x55c/0xb50 (unreliable) > [c00000001c753600] [c0000000008bb214] blk_stat_alloc_callback+0x94/0x150 > [c00000001c753650] [c0000000008b7a04] > blk_mq_init_allocated_queue+0x64/0x560 [c00000001c7536b0] > [c0000000008b8024] blk_mq_init_queue+0x54/0xb0 [c00000001c7536e0] > [c000000000b87650] scsi_mq_alloc_queue+0x30/0xa0 [c00000001c753710] > [c000000000b88b2c] scsi_alloc_sdev+0x1cc/0x300 [c00000001c7537b0] > [c000000000b897b0] scsi_probe_and_add_lun+0xb50/0x1020 [c00000001c753950] > [c000000000b8a35c] __scsi_scan_target+0x17c/0x790 [c00000001c753a80] > [c000000000b8ab90] scsi_scan_channel+0x90/0xe0 [c00000001c753ad0] > [c000000000b8ae48] scsi_scan_host_selected+0x148/0x1f0 [c00000001c753b60] > [c000000000b8b31c] do_scan_async+0x2c/0x2a0 > [c00000001c753be0] [c000000000187a18] async_run_entry_fn+0x78/0x220 > [c00000001c753c70] [c000000000176a74] process_one_work+0x264/0x540 > [c00000001c753d10] [c000000000177338] worker_thread+0xa8/0x600 > [c00000001c753da0] [c0000000001807b0] kthread+0x190/0x1a0 > [c00000001c753e10] [c00000000000d8f0] ret_from_kernel_thread+0x5c/0x6c > > With KUAP enabled the kernel uses storage key 3 for all its translations. > But as shown by the debug print, in this specific case we have the hash > page table entry created with key value 0. > > [ 2.249497] Found HPTE: v = 0x40070adbb6fffc05 r = 0x1ffffffffff1194 > > and DSISR indicates a key fault. > > This can happen due to parallel fault on the same EA by different CPUs > > CPU 0 CPU 1 > fault on X > > H_PAGE_BUSY set > fault on X > > finish fault handling and > clear H_PAGE_BUSY > check for H_PAGE_BUSY > continue with fault handling. > > This implies CPU1 will end up calling hpte_updatepp for address X > and the kernel updated the hash pte entry with key 0 > > Fixes: d94b827e89dc ("powerpc/book3s64/kuap: Use Key 3 for kernel mapping > with hash translation") > > Debugged-by: Michael Ellerman <mpe@ellerman.id.au> > Reported-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > --- I've tested this on top of commit db24726bfefa68c606947a86132591568a06bfb4 ("Merge tag 'integrity-v5.12-fix' of git://git.kernel.org/pub/scm/linux/ kernel/git/zohar/linux-integrity"), and the reported issue did not manifest. Thank you, Michael and Aneesh, for the help. Tested-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> -- Murilo
On Fri, 26 Mar 2021 12:37:55 +0530, Aneesh Kumar K.V wrote: > H_PROTECT expect the flag value to include > flags: AVPN, pp0, pp1, pp2, key0-key4, Noexec, CMO Option flags > > This patch updates hpte_updatepp() to fetch the storage key value from the linux page > table and use the same in H_PROTECT hcall. > > native_hpte_updatepp() is not updated because the kernel doesn't clear the existing > storage key value there. The kernel also doesn't use hpte_updatepp() callback for > updating storage keys. > > [...] Applied to powerpc/fixes. [1/1] powerpc/mm/book3s64: Use the correct storage key value when calling H_PROTECT https://git.kernel.org/powerpc/c/53f1d31708f6240e4615b0927df31f182e389e2f cheers
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c index 764170fdb0f7..3805519a6469 100644 --- a/arch/powerpc/platforms/pseries/lpar.c +++ b/arch/powerpc/platforms/pseries/lpar.c @@ -887,7 +887,8 @@ static long pSeries_lpar_hpte_updatepp(unsigned long slot, want_v = hpte_encode_avpn(vpn, psize, ssize); - flags = (newpp & 7) | H_AVPN; + flags = (newpp & (HPTE_R_PP | HPTE_R_N | HPTE_R_KEY_LO)) | H_AVPN; + flags |= (newpp & HPTE_R_KEY_HI) >> 48; if (mmu_has_feature(MMU_FTR_KERNEL_RO)) /* Move pp0 into bit 8 (IBM 55) */ flags |= (newpp & HPTE_R_PP0) >> 55;
H_PROTECT expect the flag value to include flags: AVPN, pp0, pp1, pp2, key0-key4, Noexec, CMO Option flags This patch updates hpte_updatepp() to fetch the storage key value from the linux page table and use the same in H_PROTECT hcall. native_hpte_updatepp() is not updated because the kernel doesn't clear the existing storage key value there. The kernel also doesn't use hpte_updatepp() callback for updating storage keys. This fixes the below kernel crash observed with KUAP enabled. BUG: Unable to handle kernel data access on write at 0xc009fffffc440000 Faulting instruction address: 0xc0000000000b7030 Key fault AMR: 0xfcffffffffffffff IAMR: 0xc0000077bc498100 Found HPTE: v = 0x40070adbb6fffc05 r = 0x1ffffffffff1194 Oops: Kernel access of bad area, sig: 11 [#1] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries ......... CFAR: c000000000010100 DAR: c009fffffc440000 DSISR: 02200000 IRQMASK: 0 .......... NIP [c0000000000b7030] memset+0x68/0x104 LR [c0000000003ef00c] pcpu_alloc+0x54c/0xb50 Call Trace: [c00000001c7534f0] [c0000000003ef01c] pcpu_alloc+0x55c/0xb50 (unreliable) [c00000001c753600] [c0000000008bb214] blk_stat_alloc_callback+0x94/0x150 [c00000001c753650] [c0000000008b7a04] blk_mq_init_allocated_queue+0x64/0x560 [c00000001c7536b0] [c0000000008b8024] blk_mq_init_queue+0x54/0xb0 [c00000001c7536e0] [c000000000b87650] scsi_mq_alloc_queue+0x30/0xa0 [c00000001c753710] [c000000000b88b2c] scsi_alloc_sdev+0x1cc/0x300 [c00000001c7537b0] [c000000000b897b0] scsi_probe_and_add_lun+0xb50/0x1020 [c00000001c753950] [c000000000b8a35c] __scsi_scan_target+0x17c/0x790 [c00000001c753a80] [c000000000b8ab90] scsi_scan_channel+0x90/0xe0 [c00000001c753ad0] [c000000000b8ae48] scsi_scan_host_selected+0x148/0x1f0 [c00000001c753b60] [c000000000b8b31c] do_scan_async+0x2c/0x2a0 [c00000001c753be0] [c000000000187a18] async_run_entry_fn+0x78/0x220 [c00000001c753c70] [c000000000176a74] process_one_work+0x264/0x540 [c00000001c753d10] [c000000000177338] worker_thread+0xa8/0x600 [c00000001c753da0] [c0000000001807b0] kthread+0x190/0x1a0 [c00000001c753e10] [c00000000000d8f0] ret_from_kernel_thread+0x5c/0x6c With KUAP enabled the kernel uses storage key 3 for all its translations. But as shown by the debug print, in this specific case we have the hash page table entry created with key value 0. [ 2.249497] Found HPTE: v = 0x40070adbb6fffc05 r = 0x1ffffffffff1194 and DSISR indicates a key fault. This can happen due to parallel fault on the same EA by different CPUs CPU 0 CPU 1 fault on X H_PAGE_BUSY set fault on X finish fault handling and clear H_PAGE_BUSY check for H_PAGE_BUSY continue with fault handling. This implies CPU1 will end up calling hpte_updatepp for address X and the kernel updated the hash pte entry with key 0 Fixes: d94b827e89dc ("powerpc/book3s64/kuap: Use Key 3 for kernel mapping with hash translation") Debugged-by: Michael Ellerman <mpe@ellerman.id.au> Reported-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> --- Similar change for bolted hash pte entries was done by https://lore.kernel.org/linuxppc-dev/20210211135130.3474832-2-mpe@ellerman.id.au/ arch/powerpc/platforms/pseries/lpar.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)