| Message ID | 20201125051634.509286-17-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Kernel userspace access/execution prevention with hash translation | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (4c202167192a77481310a3cacae9f12618b92216) |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 1 warnings, 0 checks, 72 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
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/ > + } > + /* > + * We don't want to WARN here because userspace can setup > + * keys such that a kernel access to user address can cause > + * fault > + */ > + return !!(error_code & DSISR_KEYFAULT); > } > > static __always_inline void allow_user_access(void __user *to, const void __user *from, > diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h > index a06e50b68d40..952be0414f43 100644 > --- a/arch/powerpc/include/asm/kup.h > +++ b/arch/powerpc/include/asm/kup.h > @@ -59,8 +59,8 @@ void setup_kuap(bool disabled); > #else > static inline void setup_kuap(bool disabled) { } > > -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) > { > return false; > } > diff --git a/arch/powerpc/include/asm/nohash/32/kup-8xx.h b/arch/powerpc/include/asm/nohash/32/kup-8xx.h > index 567cdc557402..7bdd9e5b63ed 100644 > --- a/arch/powerpc/include/asm/nohash/32/kup-8xx.h > +++ b/arch/powerpc/include/asm/nohash/32/kup-8xx.h > @@ -60,8 +60,8 @@ static inline void restore_user_access(unsigned long flags) > mtspr(SPRN_MD_AP, flags); > } > > -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) > { > return WARN(!((regs->kuap ^ MD_APG_KUAP) & 0xff000000), > "Bug: fault blocked by AP register !"); > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c > index 0add963a849b..c91621df0c61 100644 > --- a/arch/powerpc/mm/fault.c > +++ b/arch/powerpc/mm/fault.c > @@ -227,7 +227,7 @@ static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code, > > // Read/write fault in a valid region (the exception table search passed > // above), but blocked by KUAP is bad, it can never succeed. > - if (bad_kuap_fault(regs, address, is_write)) > + if (bad_kuap_fault(regs, address, is_write, error_code)) > return true; > > // What's left? Kernel fault on user in well defined regions (extable >
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. -aneesh
"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. cheers
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
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"); + } + /* + * We don't want to WARN here because userspace can setup + * keys such that a kernel access to user address can cause + * fault + */ + return !!(error_code & DSISR_KEYFAULT); } static __always_inline void allow_user_access(void __user *to, const void __user *from, diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h index a06e50b68d40..952be0414f43 100644 --- a/arch/powerpc/include/asm/kup.h +++ b/arch/powerpc/include/asm/kup.h @@ -59,8 +59,8 @@ void setup_kuap(bool disabled); #else static inline void setup_kuap(bool disabled) { } -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) { return false; } diff --git a/arch/powerpc/include/asm/nohash/32/kup-8xx.h b/arch/powerpc/include/asm/nohash/32/kup-8xx.h index 567cdc557402..7bdd9e5b63ed 100644 --- a/arch/powerpc/include/asm/nohash/32/kup-8xx.h +++ b/arch/powerpc/include/asm/nohash/32/kup-8xx.h @@ -60,8 +60,8 @@ static inline void restore_user_access(unsigned long flags) mtspr(SPRN_MD_AP, flags); } -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) { return WARN(!((regs->kuap ^ MD_APG_KUAP) & 0xff000000), "Bug: fault blocked by AP register !"); diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index 0add963a849b..c91621df0c61 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -227,7 +227,7 @@ static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code, // Read/write fault in a valid region (the exception table search passed // above), but blocked by KUAP is bad, it can never succeed. - if (bad_kuap_fault(regs, address, is_write)) + if (bad_kuap_fault(regs, address, is_write, error_code)) return true; // What's left? Kernel fault on user in well defined regions (extable
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(-)