Re: [PATCH] powerpc/mm/book3s64: Use the correct storage key value when calling H_PROTECT

["Murilo Opsfelder Araújo" <muriloo@linux.ibm.com>]

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