All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Michael Ellerman <mpe@ellerman.id.au>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v6 16/22] powerpc/book3s64/kuap: Improve error reporting with KUAP
Date: Thu, 26 Nov 2020 11:39:04 +0100	[thread overview]
Message-ID: <0c8fc7e7-f910-ff1e-2cdc-fc93b14a648b@csgroup.eu> (raw)
In-Reply-To: <87r1ogxy8j.fsf@mpe.ellerman.id.au>



Le 26/11/2020 à 10:29, Michael Ellerman a écrit :
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>
>>> Le 25/11/2020 à 06:16, Aneesh Kumar K.V a écrit :
>>>> With hash translation use DSISR_KEYFAULT to identify a wrong access.
>>>> With Radix we look at the AMR value and type of fault.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>>> ---
>>>>    arch/powerpc/include/asm/book3s/32/kup.h     |  4 +--
>>>>    arch/powerpc/include/asm/book3s/64/kup.h     | 27 ++++++++++++++++----
>>>>    arch/powerpc/include/asm/kup.h               |  4 +--
>>>>    arch/powerpc/include/asm/nohash/32/kup-8xx.h |  4 +--
>>>>    arch/powerpc/mm/fault.c                      |  2 +-
>>>>    5 files changed, 29 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h
>>>> index 32fd4452e960..b18cd931e325 100644
>>>> --- a/arch/powerpc/include/asm/book3s/32/kup.h
>>>> +++ b/arch/powerpc/include/asm/book3s/32/kup.h
>>>> @@ -177,8 +177,8 @@ static inline void restore_user_access(unsigned long flags)
>>>>    		allow_user_access(to, to, end - addr, KUAP_READ_WRITE);
>>>>    }
>>>>    
>>>> -static inline bool
>>>> -bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
>>>> +static inline bool bad_kuap_fault(struct pt_regs *regs, unsigned long address,
>>>> +				  bool is_write, unsigned long error_code)
>>>>    {
>>>>    	unsigned long begin = regs->kuap & 0xf0000000;
>>>>    	unsigned long end = regs->kuap << 28;
>>>> diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
>>>> index 4a3d0d601745..2922c442a218 100644
>>>> --- a/arch/powerpc/include/asm/book3s/64/kup.h
>>>> +++ b/arch/powerpc/include/asm/book3s/64/kup.h
>>>> @@ -301,12 +301,29 @@ static inline void set_kuap(unsigned long value)
>>>>    	isync();
>>>>    }
>>>>    
>>>> -static inline bool
>>>> -bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
>>>> +#define RADIX_KUAP_BLOCK_READ	UL(0x4000000000000000)
>>>> +#define RADIX_KUAP_BLOCK_WRITE	UL(0x8000000000000000)
>>>> +
>>>> +static inline bool bad_kuap_fault(struct pt_regs *regs, unsigned long address,
>>>> +				  bool is_write, unsigned long error_code)
>>>>    {
>>>> -	return WARN(mmu_has_feature(MMU_FTR_KUAP) &&
>>>> -		    (regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE : AMR_KUAP_BLOCK_READ)),
>>>> -		    "Bug: %s fault blocked by AMR!", is_write ? "Write" : "Read");
>>>> +	if (!mmu_has_feature(MMU_FTR_KUAP))
>>>> +		return false;
>>>> +
>>>> +	if (radix_enabled()) {
>>>> +		/*
>>>> +		 * Will be a storage protection fault.
>>>> +		 * Only check the details of AMR[0]
>>>> +		 */
>>>> +		return WARN((regs->kuap & (is_write ? RADIX_KUAP_BLOCK_WRITE : RADIX_KUAP_BLOCK_READ)),
>>>> +			    "Bug: %s fault blocked by AMR!", is_write ? "Write" : "Read");
>>>
>>> I think it is pointless to keep the WARN() here.
>>>
>>> I have a series aiming at removing them. See
>>> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/cc9129bdda1dbc2f0a09cf45fece7d0b0e690784.1605541983.git.christophe.leroy@csgroup.eu/
>>
>> Can we do this as a spearate patch as you posted above? We can drop the
>> WARN in that while keeping the hash branch to look at DSISR value.
> 
> Yeah we can reconcile Christophe's series with yours later.
> 
> I'm still not 100% convinced I want to drop that WARN.

Ok, you can still take the rest of the series as that patch is the last one.

But, I really can't see the point with the WARN. When I hip a kuap bad fault, I get a double dump 
(see below). The second one is the interesting one, it tells me everything about the fault. But the 
WARN provides internals of do_page_fault() function. What interesting information do I get from there ?

[   37.842509] lkdtm: attempting bad write at b7bae000
[   37.842526] ------------[ cut here ]------------
[   37.842536] Bug: write fault blocked by segment registers !
[   37.842598] WARNING: CPU: 0 PID: 434 at arch/powerpc/include/asm/book3s/32/kup.h:189 
do_page_fault+0x3c8/0x5f0
[   37.842630] CPU: 0 PID: 434 Comm: busybox Not tainted 5.10.0-rc5-s3k-dev-01343-g8bec80f73baa #4165
[   37.842650] NIP:  c00155e4 LR: c00155e4 CTR: 00000000
[   37.842670] REGS: e6719c78 TRAP: 0700   Not tainted  (5.10.0-rc5-s3k-dev-01343-g8bec80f73baa)
[   37.842683] MSR:  00021032 <ME,IR,DR,RI>  CR: 22002224  XER: 20000000
[   37.842750]
[   37.842750] GPR00: c00155e4 e6719d30 c113c660 0000002f c097adf8 c097af10 00000027 00000027
[   37.842750] GPR08: c0b0afbc 00000000 00000023 00000001 24002224 100d166a 100a0920 00000000
[   37.842750] GPR16: 100cac0c 100b0000 10169444 1016a685 100d0000 100d0000 00000000 100a0900
[   37.842750] GPR24: ffffffef ffffffff c1392220 00000300 c076f424 02000000 b7bae000 e6719d70
[   37.843049] NIP [c00155e4] do_page_fault+0x3c8/0x5f0
[   37.843074] LR [c00155e4] do_page_fault+0x3c8/0x5f0
[   37.843087] Call Trace:
[   37.843114] [e6719d30] [c00155e4] do_page_fault+0x3c8/0x5f0 (unreliable)
[   37.843154] [e6719d60] [c0014384] handle_page_fault+0x10/0x3c
[   37.843211] --- interrupt: 301 at lkdtm_ACCESS_USERSPACE+0xdc/0xe4
[   37.843211]     LR = lkdtm_ACCESS_USERSPACE+0xd0/0xe4
[   37.843238] [e6719e48] [c039d76c] direct_entry+0xe0/0x164
[   37.843281] [e6719e68] [c0286730] full_proxy_write+0x78/0xbc
[   37.843325] [e6719e88] [c01657a8] vfs_write+0xdc/0x458
[   37.843359] [e6719f08] [c0165cb0] ksys_write+0x6c/0x11c
[   37.843397] [e6719f38] [c0014164] ret_from_syscall+0x0/0x34
[   37.843426] --- interrupt: c01 at 0xfd55784
[   37.843426]     LR = 0xfe16244
[   37.843438] Instruction dump:
[   37.843459] 38600007 4bff7a19 3bc00000 4bfffdbc 419e0110 813f00b0 55280006 7c1e4040
[   37.843529] 408000f4 3c60c080 3863e148 4801552d <0fe00000> 3c80c072 3c60c097 38840d84
[   37.843602] ---[ end trace 29c115c8ef352681 ]---
[   37.843627] Kernel attempted to write user page (b7bae000) - exploit attempt? (uid: 0)
[   37.851531] BUG: Unable to handle kernel data access on write at 0xb7bae000
[   37.858472] Faulting instruction address: 0xc039e550
[   37.863432] Oops: Kernel access of bad area, sig: 11 [#1]
[   37.868822] BE PAGE_SIZE=4K PREEMPT CMPCPRO
[   37.873029] SAF3000 DIE NOTIFICATION
[   37.876624] CPU: 0 PID: 434 Comm: busybox Tainted: G        W 
5.10.0-rc5-s3k-dev-01343-g8bec80f73baa #4165
[   37.886940] NIP:  c039e550 LR: c039e544 CTR: 00000000
[   37.891988] REGS: e6719d70 TRAP: 0300   Tainted: G        W 
(5.10.0-rc5-s3k-dev-01343-g8bec80f73baa)
[   37.901866] MSR:  00009032 <EE,ME,IR,DR,RI>  CR: 24002224  XER: 00000000
[   37.908617] DAR: b7bae000 DSISR: 0a000000
[   37.908617] GPR00: c039e544 e6719e28 c113c660 c083aad8 c097adf8 c097af10 00000027 00000027
[   37.908617] GPR08: c0b0afbc c0dec0de 00000023 00000001 28002224 100d166a 100a0920 00000000
[   37.908617] GPR16: 100cac0c 100b0000 10169444 1016a685 100d0000 100d0000 00000000 100a0900
[   37.908617] GPR24: ffffffef ffffffff e6719f10 00000011 c076f424 c1cb1000 c0839e24 b7bae000
[   37.946267] NIP [c039e550] lkdtm_ACCESS_USERSPACE+0xdc/0xe4
[   37.951842] LR [c039e544] lkdtm_ACCESS_USERSPACE+0xd0/0xe4
[   37.957316] Call Trace:
[   37.959782] [e6719e28] [c039e544] lkdtm_ACCESS_USERSPACE+0xd0/0xe4 (unreliable)
[   37.967102] [e6719e48] [c039d76c] direct_entry+0xe0/0x164
[   37.972524] [e6719e68] [c0286730] full_proxy_write+0x78/0xbc
[   37.978204] [e6719e88] [c01657a8] vfs_write+0xdc/0x458
[   37.983358] [e6719f08] [c0165cb0] ksys_write+0x6c/0x11c
[   37.988605] [e6719f38] [c0014164] ret_from_syscall+0x0/0x34
[   37.994185] --- interrupt: c01 at 0xfd55784
[   37.994185]     LR = 0xfe16244
[   38.001385] Instruction dump:
[   38.004360] 3863ac00 3d29c0df 3929c0de 91210008 4bcd04c9 3c60c084 3863ac24 7fe4fb78
[   38.012149] 4bcd04b9 3c60c084 81210008 3863aad8 <913f0000> 4bffff80 3c60c084 3863aa00
[   38.020120] ---[ end trace 29c115c8ef352682 ]---

Christophe

  reply	other threads:[~2020-11-26 10:40 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-25  5:16 [PATCH v6 00/22] Kernel userspace access/execution prevention with hash translation Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 01/22] powerpc: Add new macro to handle NESTED_IFCLR Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 02/22] KVM: PPC: BOOK3S: PR: Ignore UAMOR SPR Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 03/22] powerpc/book3s64/kuap/kuep: Make KUAP and KUEP a subfeature of PPC_MEM_KEYS Aneesh Kumar K.V
2020-11-25 13:30   ` Christophe Leroy
2020-11-25 14:57     ` Aneesh Kumar K.V
2020-11-26  3:25   ` Michael Ellerman
2020-11-25  5:16 ` [PATCH v6 04/22] powerpc/book3s64/kuap/kuep: Move uamor setup to pkey init Aneesh Kumar K.V
2020-11-25 13:32   ` Christophe Leroy
2020-11-26  3:28   ` Michael Ellerman
2020-11-25  5:16 ` [PATCH v6 05/22] powerpc/book3s64/kuap: Move KUAP related function outside radix Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 06/22] powerpc/book3s64/kuep: Move KUEP " Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 07/22] powerpc/book3s64/kuap: Rename MMU_FTR_RADIX_KUAP to MMU_FTR_KUAP Aneesh Kumar K.V
2020-11-25 13:43   ` Christophe Leroy
2020-11-25 13:52     ` Aneesh Kumar K.V
2020-11-26  3:16   ` Michael Ellerman
2020-11-25  5:16 ` [PATCH v6 08/22] powerpc/book3s64/kuap: Use Key 3 for kernel mapping with hash translation Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 09/22] powerpc/exec: Set thread.regs early during exec Aneesh Kumar K.V
2020-11-25 13:47   ` Christophe Leroy
2020-11-26  7:38     ` Aneesh Kumar K.V
2020-11-26  7:43       ` Christophe Leroy
2020-11-25  5:16 ` [PATCH v6 10/22] powerpc/book3s64/pkeys: Store/restore userspace AMR/IAMR correctly on entry and exit from kernel Aneesh Kumar K.V
2020-11-25 13:52   ` Christophe Leroy
2020-11-25 13:55     ` Aneesh Kumar K.V
2020-11-25 14:16       ` Christophe Leroy
2020-11-25  5:16 ` [PATCH v6 11/22] powerpc/book3s64/pkeys: Inherit correctly on fork Aneesh Kumar K.V
2020-11-25 13:54   ` Christophe Leroy
2020-11-25 13:56     ` Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 12/22] powerpc/book3s64/pkeys: Reset userspace AMR correctly on exec Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 13/22] powerpc/ptrace-view: Use pt_regs values instead of thread_struct based one Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 14/22] powerpc/book3s64/pkeys: Don't update SPRN_AMR when in kernel mode Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 15/22] powerpc/book3s64/kuap: Restrict access to userspace based on userspace AMR Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 16/22] powerpc/book3s64/kuap: Improve error reporting with KUAP Aneesh Kumar K.V
2020-11-25 14:04   ` Christophe Leroy
2020-11-26  7:44     ` Aneesh Kumar K.V
2020-11-26  9:29       ` Michael Ellerman
2020-11-26 10:39         ` Christophe Leroy [this message]
2020-11-25  5:16 ` [PATCH v6 17/22] powerpc/book3s64/kuap: Use Key 3 to implement KUAP with hash translation Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 18/22] powerpc/book3s64/kuep: Use Key 3 to implement KUEP " Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 19/22] powerpc/book3s64/hash/kuap: Enable kuap on hash Aneesh Kumar K.V
2021-03-15 12:06   ` Christophe Leroy
2021-03-15 12:59     ` Aneesh Kumar K.V
2021-03-15 13:02       ` Christophe Leroy
2021-10-08  9:32   ` Christophe Leroy
2021-10-11  3:28     ` Michael Ellerman
2020-11-25  5:16 ` [PATCH v6 20/22] powerpc/book3s64/hash/kuep: Enable KUEP " Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 21/22] powerpc/book3s64/hash/kup: Don't hardcode kup key Aneesh Kumar K.V
2020-11-25  5:16 ` [PATCH v6 22/22] powerpc/book3s64/pkeys: Optimize FTR_KUAP and FTR_KUEP disabled case Aneesh Kumar K.V

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0c8fc7e7-f910-ff1e-2cdc-fc93b14a648b@csgroup.eu \
    --to=christophe.leroy@csgroup.eu \
    --cc=aneesh.kumar@linux.ibm.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.