This patch series implements KUAP and KUEP with hash translation mode using
memory keys. The kernel now uses memory protection key 3 to control access
to the kernel. Kernel page table entries are now configured with key 3.
Access to locations configured with any other key value is denied when in
kernel mode (MSR_PR=0). This includes userspace which is by default configured
with key 0.
Changes from v3:
* Fix build error reported by kernel test robot [off-list ref]
Changes from v2:
* Rebase to the latest kernel.
* Fixed a bug with disabling KUEP/KUAP on kernel command line
* Added a patch to make kup key dynamic.
Changes from V1:
* Rebased on latest kernel
Aneesh Kumar K.V (41):
powerpc/book3s64/pkeys: Fixup bit numbering
powerpc/book3s64/pkeys: pkeys are supported only on hash on book3s.
powerpc/book3s64/pkeys: Move pkey related bits in the linux page table
powerpc/book3s64/pkeys: Explain key 1 reservation details
powerpc/book3s64/pkeys: Simplify the key initialization
powerpc/book3s64/pkeys: Prevent key 1 modification from userspace.
powerpc/book3s64/pkeys: kill cpu feature key CPU_FTR_PKEY
powerpc/book3s64/pkeys: Convert execute key support to static key
powerpc/book3s64/pkeys: Simplify pkey disable branch
powerpc/book3s64/pkeys: Convert pkey_total to max_pkey
powerpc/book3s64/pkeys: Make initial_allocation_mask static
powerpc/book3s64/pkeys: Mark all the pkeys above max pkey as reserved
powerpc/book3s64/pkeys: Enable MMU_FTR_PKEY
powerpc/book3s64/kuep: Add MMU_FTR_KUEP
powerpc/book3s64/pkeys: Use execute_pkey_disable static key
powerpc/book3s64/pkeys: Use MMU_FTR_PKEY instead of pkey_disabled
static key
powerpc/book3s64/kuap: Move KUAP related function outside radix
powerpc/book3s64/kuep: Move KUEP related function outside radix
powerpc/book3s64/kuap: Rename MMU_FTR_RADIX_KUAP to MMU_FTR_KUAP
powerpc/book3s64/kuap/kuep: Make KUAP and KUEP a subfeature of
PPC_MEM_KEYS
powerpc/book3s64/kuap: Move UAMOR setup to key init function
powerpc/book3s64/kuap: Use Key 3 for kernel mapping with hash
translation
powerpc/exec: Set thread.regs early during exec
powerpc/book3s64/pkeys: Store/restore userspace AMR correctly on entry
and exit from kernel
powerpc/book3s64/kuep: Store/restore userspace IAMR correctly on entry
and exit from kernel
powerpc/book3s64/pkeys: Inherit correctly on fork.
powerpc/book3s64/pkeys: Reset userspace AMR correctly on exec
powerpc/ptrace-view: Use pt_regs values instead of thread_struct based
one.
powerpc/book3s64/pkeys: Don't update SPRN_AMR when in kernel mode.
powerpc/book3s64/kuap: Restrict access to userspace based on userspace
AMR
powerpc/book3s64/kuap: Improve error reporting with KUAP
powerpc/book3s64/kuap: Use Key 3 to implement KUAP with hash
translation.
powerpc/book3s64/kuep: Use Key 3 to implement KUEP with hash
translation.
powerpc/book3s64/hash/kuap: Enable kuap on hash
powerpc/book3s64/hash/kuep: Enable KUEP on hash
powerpc/book3s64/keys: Print information during boot.
powerpc/selftest/ptrave-pkey: Rename variables to make it easier to
follow code
powerpc/selftest/ptrace-pkey: Update the test to mark an invalid pkey
correctly
powerpc/selftest/ptrace-pkey: IAMR and uamor cannot be updated by
ptrace
powerpc/book3s64/keys/kuap: Reset AMR/IAMR values on kexec
powerpc/book3s64/hash/kup: Don't hardcode kup key
arch/powerpc/include/asm/book3s/32/kup.h | 4 +-
arch/powerpc/include/asm/book3s/64/hash-4k.h | 21 +-
arch/powerpc/include/asm/book3s/64/hash-64k.h | 12 +-
.../powerpc/include/asm/book3s/64/hash-pkey.h | 24 +
arch/powerpc/include/asm/book3s/64/hash.h | 3 +-
.../powerpc/include/asm/book3s/64/kup-radix.h | 185 --------
arch/powerpc/include/asm/book3s/64/kup.h | 391 +++++++++++++++++
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 9 +-
arch/powerpc/include/asm/book3s/64/mmu.h | 6 +
arch/powerpc/include/asm/book3s/64/pgtable.h | 17 +-
arch/powerpc/include/asm/book3s/64/pkeys.h | 25 ++
arch/powerpc/include/asm/cputable.h | 13 +-
arch/powerpc/include/asm/kup.h | 20 +-
arch/powerpc/include/asm/mmu.h | 17 +-
arch/powerpc/include/asm/mmu_context.h | 2 +-
arch/powerpc/include/asm/nohash/32/kup-8xx.h | 4 +-
arch/powerpc/include/asm/pkeys.h | 65 +--
arch/powerpc/include/asm/processor.h | 5 -
arch/powerpc/include/asm/ptrace.h | 8 +-
arch/powerpc/include/asm/thread_info.h | 2 -
arch/powerpc/kernel/asm-offsets.c | 6 +-
arch/powerpc/kernel/dt_cpu_ftrs.c | 6 -
arch/powerpc/kernel/entry_64.S | 6 +-
arch/powerpc/kernel/exceptions-64s.S | 4 +-
arch/powerpc/kernel/misc_64.S | 14 -
arch/powerpc/kernel/process.c | 56 ++-
arch/powerpc/kernel/prom.c | 5 +
arch/powerpc/kernel/ptrace/ptrace-view.c | 23 +-
arch/powerpc/kernel/smp.c | 5 +
arch/powerpc/kernel/syscall_64.c | 32 +-
arch/powerpc/kernel/traps.c | 6 -
arch/powerpc/kexec/core_64.c | 3 +
arch/powerpc/mm/book3s64/hash_4k.c | 2 +-
arch/powerpc/mm/book3s64/hash_64k.c | 4 +-
arch/powerpc/mm/book3s64/hash_hugepage.c | 2 +-
arch/powerpc/mm/book3s64/hash_hugetlbpage.c | 2 +-
arch/powerpc/mm/book3s64/hash_pgtable.c | 2 +-
arch/powerpc/mm/book3s64/hash_utils.c | 10 +-
arch/powerpc/mm/book3s64/pgtable.c | 3 +
arch/powerpc/mm/book3s64/pkeys.c | 412 +++++++++++-------
arch/powerpc/mm/book3s64/radix_pgtable.c | 36 --
arch/powerpc/mm/fault.c | 2 +-
arch/powerpc/platforms/Kconfig.cputype | 4 +-
.../selftests/powerpc/ptrace/ptrace-pkey.c | 53 +--
44 files changed, 939 insertions(+), 592 deletions(-)
create mode 100644 arch/powerpc/include/asm/book3s/64/hash-pkey.h
delete mode 100644 arch/powerpc/include/asm/book3s/64/kup-radix.h
create mode 100644 arch/powerpc/include/asm/book3s/64/kup.h
create mode 100644 arch/powerpc/include/asm/book3s/64/pkeys.h
--
2.26.2
This number the pkey bit such that it is easy to follow. PKEY_BIT0 is
the lower order bit. This makes further changes easy to follow.
No functional change in this patch other than linux page table for
hash translation now maps pkeys differently.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/hash-4k.h | 9 +++----
arch/powerpc/include/asm/book3s/64/hash-64k.h | 8 +++----
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 8 +++----
arch/powerpc/include/asm/pkeys.h | 24 +++++++++----------
4 files changed, 25 insertions(+), 24 deletions(-)
@@ -124,7 +124,10 @@ static int pkey_initialize(void)#elseos_reserved=0;#endif-/* Bits are in LE format. */+/*+*key1isrecommendednottobeused.PowerISA(3.0)page1015,+*programmingnote.+*/reserved_allocation_mask=(0x1<<1)|(0x1<<execute_only_key);/* register mask is in BE format */
Add documentation explaining the execute_only_key. The reservation and initialization mask
details are also explained in this patch.
No functional change in this patch.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/mm/book3s64/pkeys.c | 186 ++++++++++++++++++-------------
1 file changed, 107 insertions(+), 79 deletions(-)
@@ -15,48 +15,71 @@DEFINE_STATIC_KEY_TRUE(pkey_disabled);intpkeys_total;/* Total pkeys as per device tree */u32initial_allocation_mask;/* Bits set for the initially allocated keys */-u32reserved_allocation_mask;/* Bits set for reserved keys */+/*+*Keysmarkedinthereservationlistcannotbeallocatedbyuserspace+*/+u32reserved_allocation_mask;staticboolpkey_execute_disable_supported;-staticboolpkeys_devtree_defined;/* property exported by device tree */-staticu64pkey_amr_mask;/* Bits in AMR not to be touched */-staticu64pkey_iamr_mask;/* Bits in AMR not to be touched */-staticu64pkey_uamor_mask;/* Bits in UMOR not to be touched */+staticu64default_amr;+staticu64default_iamr;+/* Allow all keys to be modified by default */+staticu64default_uamor=~0x0UL;+/*+*KeyusedtoimplementPROT_EXECmmap.DeniesREAD/WRITE+*Wepickkey2because0isspecialkeyand1isreservedasperISA.+*/staticintexecute_only_key=2;+#define AMR_BITS_PER_PKEY 2#define AMR_RD_BIT 0x1UL#define AMR_WR_BIT 0x2UL#define IAMR_EX_BIT 0x1UL-#define PKEY_REG_BITS (sizeof(u64)*8)+#define PKEY_REG_BITS (sizeof(u64) * 8)#define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY))-staticvoidscan_pkey_feature(void)+staticintscan_pkey_feature(void){u32vals[2];+intpkeys_total=0;structdevice_node*cpu;+/*+*PkeyisnotsupportedwithRadixtranslation.+*/+if(radix_enabled())+return0;+cpu=of_find_node_by_type(NULL,"cpu");if(!cpu)-return;+return0;if(of_property_read_u32_array(cpu,-"ibm,processor-storage-keys",vals,2))-return;+"ibm,processor-storage-keys",vals,2)==0){+/*+*Sinceanypkeycanbeusedfordataorexecute,wewill+*justtreatallkeysasequalandtrackthemasoneentity.+*/+pkeys_total=vals[0];+/* Should we check for IAMR support FIXME!! */+}else{+/*+*Let'sassume32pkeysonP8baremetal,ifitsnotdefinedbydevice+*tree.Wemakethisexceptionsinceskibootforgottoexposethis+*propertyonpower8.+*/+if(!firmware_has_feature(FW_FEATURE_LPAR)&&+cpu_has_feature(CPU_FTRS_POWER8))+pkeys_total=32;+}/*-*Sinceanypkeycanbeusedfordataorexecute,wewilljusttreat-*allkeysasequalandtrackthemasoneentity.+*Adjusttheupperlimit,basedonthenumberofbitssupportedby+*arch-neutralcode.*/-pkeys_total=vals[0];-pkeys_devtree_defined=true;-}--staticinlineboolpkey_mmu_enabled(void)-{-if(firmware_has_feature(FW_FEATURE_LPAR))-returnpkeys_total;-else-returncpu_has_feature(CPU_FTR_PKEY);+pkeys_total=min_t(int,pkeys_total,+((ARCH_VM_PKEY_FLAGS>>VM_PKEY_SHIFT)+1));+returnpkeys_total;}staticintpkey_initialize(void)
@@ -80,31 +103,13 @@ static int pkey_initialize(void)!=(sizeof(u64)*BITS_PER_BYTE));/* scan the device tree for pkey feature */-scan_pkey_feature();--/*-*Let'sassume32pkeysonP8baremetal,ifitsnotdefinedbydevice-*tree.Wemakethisexceptionsinceskibootforgottoexposethis-*propertyonpower8.-*/-if(!pkeys_devtree_defined&&!firmware_has_feature(FW_FEATURE_LPAR)&&-cpu_has_feature(CPU_FTRS_POWER8))-pkeys_total=32;--/*-*Adjusttheupperlimit,basedonthenumberofbitssupportedby-*arch-neutralcode.-*/-pkeys_total=min_t(int,pkeys_total,-((ARCH_VM_PKEY_FLAGS>>VM_PKEY_SHIFT)+1));--if(!pkey_mmu_enabled()||radix_enabled()||!pkeys_total)-static_branch_enable(&pkey_disabled);-else+pkeys_total=scan_pkey_feature();+if(pkeys_total)static_branch_disable(&pkey_disabled);--if(static_branch_likely(&pkey_disabled))+else{+static_branch_enable(&pkey_disabled);return0;+}/**Thedevicetreecannotbereliedtoindicatesupportfor
@@ -118,48 +123,71 @@ static int pkey_initialize(void)#ifdef CONFIG_PPC_4K_PAGES/**TheOScanmanageonly8pkeysduetoitsinabilitytorepresentthem-*intheLinux4KPTE.+*intheLinux4KPTE.Markallotherkeysreserved.*/os_reserved=pkeys_total-8;#elseos_reserved=0;#endif-/*-*key1isrecommendednottobeused.PowerISA(3.0)page1015,-*programmingnote.-*/-reserved_allocation_mask=(0x1<<1)|(0x1<<execute_only_key);--/* register mask is in BE format */-pkey_amr_mask=~0x0ul;-pkey_amr_mask&=~(0x3ul<<pkeyshift(0));--pkey_iamr_mask=~0x0ul;-pkey_iamr_mask&=~(0x3ul<<pkeyshift(0));-pkey_iamr_mask&=~(0x3ul<<pkeyshift(execute_only_key));--pkey_uamor_mask=~0x0ul;-pkey_uamor_mask&=~(0x3ul<<pkeyshift(0));-pkey_uamor_mask&=~(0x3ul<<pkeyshift(execute_only_key));--/* mark the rest of the keys as reserved and hence unavailable */-for(i=(pkeys_total-os_reserved);i<pkeys_total;i++){-reserved_allocation_mask|=(0x1<<i);-pkey_uamor_mask&=~(0x3ul<<pkeyshift(i));-}-initial_allocation_mask=reserved_allocation_mask|(0x1<<0);if(unlikely((pkeys_total-os_reserved)<=execute_only_key)){/**Insufficientnumberofkeystosupport*executeonlykey.Markitunavailable.-*AnyAMR,UAMOR,IAMRbitsetfor-*thiskeyisirrelevantsincethiskey-*canneverbeallocated.*/execute_only_key=-1;+}else{+/*+*Marktheexecute_only_pkeyasnotavailablefor+*userallocationviapkey_alloc.+*/+reserved_allocation_mask|=(0x1<<execute_only_key);++/*+*DenyREAD/WRITEforexecute_only_key.+*AllowexecuteinIAMR.+*/+default_amr|=(0x3ul<<pkeyshift(execute_only_key));+default_iamr&=~(0x3ul<<pkeyshift(execute_only_key));++/*+*Cleartheuamorbitsforthiskey.+*/+default_uamor&=~(0x3ul<<pkeyshift(execute_only_key));}+/*+*Allowaccessforonlykey0.Andpreventanyothermodification.+*/+default_amr&=~(0x3ul<<pkeyshift(0));+default_iamr&=~(0x3ul<<pkeyshift(0));+default_uamor&=~(0x3ul<<pkeyshift(0));+/*+*key0isspecialinthatwewanttoconsideritanallocated+*keywhichispreallocated.Wedon'tallowchangingAMRbits+*w.r.tkey0.Butonecanpkey_free(key0)+*/+initial_allocation_mask|=(0x1<<0);++/*+*key1isrecommendednottobeused.PowerISA(3.0)page1015,+*programmingnote.+*/+reserved_allocation_mask|=(0x1<<1);++/*+*PreventtheusageofOSreservedthekeys.UpdateUAMOR+*forthosekeys.+*/+for(i=(pkeys_total-os_reserved);i<pkeys_total;i++){+reserved_allocation_mask|=(0x1<<i);+default_uamor&=~(0x3ul<<pkeyshift(i));+}+/*+*Preventtheallocationofreservedkeystoo.+*/+initial_allocation_mask|=reserved_allocation_mask;+return0;}
@@ -13,13 +13,13 @@#include<linux/of_device.h>DEFINE_STATIC_KEY_TRUE(pkey_disabled);+DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);intpkeys_total;/* Total pkeys as per device tree */u32initial_allocation_mask;/* Bits set for the initially allocated keys *//**Keysmarkedinthereservationlistcannotbeallocatedbyuserspace*/u32reserved_allocation_mask;-staticboolpkey_execute_disable_supported;staticu64default_amr;staticu64default_iamr;/* Allow all keys to be modified by default */
@@ -116,9 +116,7 @@ static int pkey_initialize(void)*execute_disablesupport.InsteadweuseaPVRcheck.*/if(pvr_version_is(PVR_POWER7)||pvr_version_is(PVR_POWER7p))-pkey_execute_disable_supported=false;-else-pkey_execute_disable_supported=true;+static_branch_enable(&execute_pkey_disabled);#ifdef CONFIG_PPC_4K_PAGES/*
@@ -282,7 +280,7 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,return-EINVAL;if(init_val&PKEY_DISABLE_EXECUTE){-if(!pkey_execute_disable_supported)+if(static_branch_unlikely(&execute_pkey_disabled))return-EINVAL;new_iamr_bits|=IAMR_EX_BIT;}
Make the default value FALSE (pkey enabled) and set to TRUE when we
find the total number of keys supported to be zero.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/pkeys.h | 2 +-
arch/powerpc/mm/book3s64/pkeys.c | 7 +++----
2 files changed, 4 insertions(+), 5 deletions(-)
@@ -11,7 +11,7 @@#include<linux/jump_label.h>#include<asm/firmware.h>-DECLARE_STATIC_KEY_TRUE(pkey_disabled);+DECLARE_STATIC_KEY_FALSE(pkey_disabled);externintpkeys_total;/* total pkeys as per device tree */externu32initial_allocation_mask;/* bits set for the initially allocated keys */externu32reserved_allocation_mask;/* bits set for reserved keys */
@@ -12,7 +12,7 @@#include<linux/pkeys.h>#include<linux/of_device.h>-DEFINE_STATIC_KEY_TRUE(pkey_disabled);+DEFINE_STATIC_KEY_FALSE(pkey_disabled);DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);intpkeys_total;/* Total pkeys as per device tree */u32initial_allocation_mask;/* Bits set for the initially allocated keys */
@@ -104,9 +104,8 @@ static int pkey_initialize(void)/* scan the device tree for pkey feature */pkeys_total=scan_pkey_feature();-if(pkeys_total)-static_branch_disable(&pkey_disabled);-else{+if(!pkeys_total){+/* No support for pkey. Mark it disabled */static_branch_enable(&pkey_disabled);return0;}
@@ -12,7 +12,7 @@#include<asm/firmware.h>DECLARE_STATIC_KEY_FALSE(pkey_disabled);-externintpkeys_total;/* total pkeys as per device tree */+externintmax_pkey;externu32initial_allocation_mask;/* bits set for the initially allocated keys */externu32reserved_allocation_mask;/* bits set for reserved keys */
@@ -14,7 +14,7 @@DEFINE_STATIC_KEY_FALSE(pkey_disabled);DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);-intpkeys_total;/* Total pkeys as per device tree */+intmax_pkey;/* Maximum key value supported */u32initial_allocation_mask;/* Bits set for the initially allocated keys *//**Keysmarkedinthereservationlistcannotbeallocatedbyuserspace
@@ -84,7 +84,7 @@ static int scan_pkey_feature(void)staticintpkey_initialize(void){-intos_reserved,i;+intpkeys_total,i;/**WedefinePKEY_DISABLE_EXECUTEinadditiontothearch-neutral
@@ -122,12 +122,12 @@ static int pkey_initialize(void)*TheOScanmanageonly8pkeysduetoitsinabilitytorepresentthem*intheLinux4KPTE.Markallotherkeysreserved.*/-os_reserved=pkeys_total-8;+max_pkey=min(8,pkeys_total);#else-os_reserved=0;+max_pkey=pkeys_total;#endif-if(unlikely((pkeys_total-os_reserved)<=execute_only_key)){+if(unlikely(max_pkey<=execute_only_key)){/**Insufficientnumberofkeystosupport*executeonlykey.Markitunavailable.
@@ -174,10 +174,10 @@ static int pkey_initialize(void)default_uamor&=~(0x3ul<<pkeyshift(1));/*-*PreventtheusageofOSreservedthekeys.UpdateUAMOR+*PreventtheusageofOSreservedkeys.UpdateUAMOR*forthosekeys.*/-for(i=(pkeys_total-os_reserved);i<pkeys_total;i++){+for(i=max_pkey;i<pkeys_total;i++){reserved_allocation_mask|=(0x1<<i);default_uamor&=~(0x3ul<<pkeyshift(i));}
@@ -15,11 +15,11 @@DEFINE_STATIC_KEY_FALSE(pkey_disabled);DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);intmax_pkey;/* Maximum key value supported */-u32initial_allocation_mask;/* Bits set for the initially allocated keys *//**Keysmarkedinthereservationlistcannotbeallocatedbyuserspace*/u32reserved_allocation_mask;+staticu32initial_allocation_mask;/* Bits set for the initially allocated keys */staticu64default_amr;staticu64default_iamr;/* Allow all keys to be modified by default */
The hypervisor can return less than max allowed pkey (for ex: 31) instead
of 32. We should mark all the pkeys above max allowed as reserved so
that we avoid the allocation of the wrong pkey(for ex: key 31 in the above
case) by userspace.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/mm/book3s64/pkeys.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Parse storage keys related device tree entry in early_init_devtree
and enable MMU feature MMU_FTR_PKEY if pkeys are supported.
MMU feature is used instead of CPU feature because this enables us
to group MMU_FTR_KUAP and MMU_FTR_PKEY in asm feature fixup code.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 6 +++
arch/powerpc/include/asm/mmu.h | 6 +++
arch/powerpc/kernel/prom.c | 5 +++
arch/powerpc/mm/book3s64/pkeys.c | 54 ++++++++++++++----------
4 files changed, 48 insertions(+), 23 deletions(-)
@@ -815,6 +815,11 @@ void __init early_init_devtree(void *params)/* Now try to figure out if we are running on LPAR and so on */pseries_probe_fw_features();+/*+*InitializepkeyfeaturesanddefaultAMR/IAMRvalues+*/+pkey_early_init_devtree();+#ifdef CONFIG_PPC_PS3/* Identify PS3 firmware */if(of_flat_dt_is_compatible(of_get_flat_dt_root(),"sony,ps3"))
@@ -38,38 +39,45 @@ static int execute_only_key = 2;#define PKEY_REG_BITS (sizeof(u64) * 8)#define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY))+staticint__initdt_scan_storage_keys(unsignedlongnode,+constchar*uname,intdepth,+void*data)+{+constchar*type=of_get_flat_dt_prop(node,"device_type",NULL);+const__be32*prop;+intpkeys_total;++/* We are scanning "cpu" nodes only */+if(type==NULL||strcmp(type,"cpu")!=0)+return0;++prop=of_get_flat_dt_prop(node,"ibm,processor-storage-keys",NULL);+if(!prop)+return0;+pkeys_total=be32_to_cpu(prop[0]);+returnpkeys_total;+}+staticintscan_pkey_feature(void){-u32vals[2];-intpkeys_total=0;-structdevice_node*cpu;+intpkeys_total;/**PkeyisnotsupportedwithRadixtranslation.*/-if(radix_enabled())+if(early_radix_enabled())return0;-cpu=of_find_node_by_type(NULL,"cpu");-if(!cpu)-return0;+pkeys_total=of_scan_flat_dt(dt_scan_storage_keys,NULL);+if(pkeys_total==0){-if(of_property_read_u32_array(cpu,-"ibm,processor-storage-keys",vals,2)==0){-/*-*Sinceanypkeycanbeusedfordataorexecute,wewill-*justtreatallkeysasequalandtrackthemasoneentity.-*/-pkeys_total=vals[0];-/* Should we check for IAMR support FIXME!! */-}else{/**Let'sassume32pkeysonP8baremetal,ifitsnotdefinedbydevice*tree.Wemakethisexceptionsinceskibootforgottoexposethis*propertyonpower8.*/if(!firmware_has_feature(FW_FEATURE_LPAR)&&-cpu_has_feature(CPU_FTRS_POWER8))+early_cpu_has_feature(CPU_FTRS_POWER8))pkeys_total=32;}
@@ -82,7 +90,7 @@ static int scan_pkey_feature(void)returnpkeys_total;}-staticintpkey_initialize(void)+void__initpkey_early_init_devtree(void){intpkeys_total,i;
@@ -107,9 +115,11 @@ static int pkey_initialize(void)if(!pkeys_total){/* No support for pkey. Mark it disabled */static_branch_enable(&pkey_disabled);-return0;+return;}+cur_cpu_spec->mmu_features|=MMU_FTR_PKEY;+/**Thedevicetreecannotbereliedtoindicatesupportfor*execute_disablesupport.InsteadweuseaPVRcheck.
@@ -187,11 +197,9 @@ static int pkey_initialize(void)*/initial_allocation_mask|=reserved_allocation_mask;-return0;+return;}-arch_initcall(pkey_initialize);-voidpkey_mm_init(structmm_struct*mm){if(static_branch_likely(&pkey_disabled))
@@ -11,7 +11,6 @@#include<linux/jump_label.h>#include<asm/firmware.h>-DECLARE_STATIC_KEY_FALSE(pkey_disabled);externintmax_pkey;externu32reserved_allocation_mask;/* bits set for reserved keys */
@@ -13,7 +13,6 @@#include<linux/of_fdt.h>-DEFINE_STATIC_KEY_FALSE(pkey_disabled);DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);intmax_pkey;/* Maximum key value supported *//*
@@ -114,7 +113,6 @@ void __init pkey_early_init_devtree(void)pkeys_total=scan_pkey_feature();if(!pkeys_total){/* No support for pkey. Mark it disabled */-static_branch_enable(&pkey_disabled);return;}
@@ -306,7 +304,7 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,voidthread_pkey_regs_save(structthread_struct*thread){-if(static_branch_likely(&pkey_disabled))+if(!mmu_has_feature(MMU_FTR_PKEY))return;/*
The next set of patches adds support for kuap with hash translation.
In preparation for that rename/move kuap related functions to
non radix names.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
.../asm/book3s/64/{kup-radix.h => kup.h} | 6 +++---
arch/powerpc/include/asm/kup.h | 2 +-
arch/powerpc/kernel/syscall_64.c | 2 +-
arch/powerpc/mm/book3s64/pkeys.c | 19 +++++++++++++++++++
arch/powerpc/mm/book3s64/radix_pgtable.c | 18 ------------------
5 files changed, 24 insertions(+), 23 deletions(-)
rename arch/powerpc/include/asm/book3s/64/{kup-radix.h => kup.h} (97%)
diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup.hsimilarity index 97%rename from arch/powerpc/include/asm/book3s/64/kup-radix.hrename to arch/powerpc/include/asm/book3s/64/kup.hindex 3ee1ec60be84..dff1fef765fa 100644--- a/arch/powerpc/include/asm/book3s/64/kup-radix.h+++ b/arch/powerpc/include/asm/book3s/64/kup.h
@@ -12,6 +12,7 @@#include<linux/pkeys.h>#include<linux/of_fdt.h>+#include<asm/smp.h>DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);intmax_pkey;/* Maximum key value supported */
@@ -198,6 +199,24 @@ void __init pkey_early_init_devtree(void)return;}+#ifdef CONFIG_PPC_KUAP+void__initsetup_kuap(booldisabled)+{+if(disabled||!early_radix_enabled())+return;++if(smp_processor_id()==boot_cpuid){+pr_info("Activating Kernel Userspace Access Prevention\n");+cur_cpu_spec->mmu_features|=MMU_FTR_RADIX_KUAP;+}++/* Make sure userspace can't change the AMR */+mtspr(SPRN_UAMOR,0);+mtspr(SPRN_AMR,AMR_KUAP_BLOCKED);+isync();+}+#endif+voidpkey_mm_init(structmm_struct*mm){if(!mmu_has_feature(MMU_FTR_PKEY))
The next set of patches adds support for kuep with hash translation.
In preparation for that rename/move kuap related functions to
non radix names.
Also set MMU_FTR_KUEP and add the missing isync().
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 1 +
arch/powerpc/mm/book3s64/pkeys.c | 21 +++++++++++++++++++++
arch/powerpc/mm/book3s64/radix_pgtable.c | 18 ------------------
3 files changed, 22 insertions(+), 18 deletions(-)
The next set of patches adds support for kuap with hash translation.
Hence make KUAP a BOOK3S_64 feature. Also make it a subfeature of
PPC_MEM_KEYS. Hash translation is going to use pkeys to support
KUAP/KUEP. Adding this dependency reduces the code complexity and
enables us to move some of the initialization code to pkeys.c
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 33 ++++++++++++++----------
arch/powerpc/include/asm/ptrace.h | 2 +-
arch/powerpc/kernel/asm-offsets.c | 2 +-
arch/powerpc/platforms/Kconfig.cputype | 4 +--
4 files changed, 23 insertions(+), 18 deletions(-)
With hash translation, the kernel will use key 3 for implementing
KUAP feature. Hence the default UAMOR value depends on what other
keys are marked reserved. Move the UAMOR initialization to pkeys init.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 2 ++
arch/powerpc/kernel/smp.c | 5 +++++
arch/powerpc/mm/book3s64/pkeys.c | 25 +++++++++++++++++++-----
3 files changed, 27 insertions(+), 5 deletions(-)
@@ -24,7 +24,7 @@ static u32 initial_allocation_mask; /* Bits set for the initially allocated kstaticu64default_amr;staticu64default_iamr;/* Allow all keys to be modified by default */-staticu64default_uamor=~0x0UL;+u64default_uamor=~0x0UL;/**KeyusedtoimplementPROT_EXECmmap.DeniesREAD/WRITE*Wepickkey2because0isspecialkeyand1isreservedasperISA.
@@ -113,8 +113,16 @@ void __init pkey_early_init_devtree(void)/* scan the device tree for pkey feature */pkeys_total=scan_pkey_feature();if(!pkeys_total){-/* No support for pkey. Mark it disabled */-return;+/*+*Nokeysupportbutonradixwecanusekey0+*toimplementkuap.+*/+if(early_radix_enabled())+/*+*Makesureuserspacecan'tchangetheAMR+*/+default_uamor=0;+gotoerr_out;}cur_cpu_spec->mmu_features|=MMU_FTR_PKEY;
This patch updates kernel hash page table entries to use storage key 3
for its mapping. This implies all kernel access will now use key 3 to
control READ/WRITE. The patch also prevents the allocation of key 3 from
userspace and UAMOR value is updated such that userspace cannot modify key 3.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
.../powerpc/include/asm/book3s/64/hash-pkey.h | 24 ++++++++++++++-----
arch/powerpc/include/asm/book3s/64/hash.h | 3 ++-
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 1 +
arch/powerpc/include/asm/mmu_context.h | 2 +-
arch/powerpc/mm/book3s64/hash_4k.c | 2 +-
arch/powerpc/mm/book3s64/hash_64k.c | 4 ++--
arch/powerpc/mm/book3s64/hash_hugepage.c | 2 +-
arch/powerpc/mm/book3s64/hash_hugetlbpage.c | 2 +-
arch/powerpc/mm/book3s64/hash_pgtable.c | 2 +-
arch/powerpc/mm/book3s64/hash_utils.c | 10 ++++----
arch/powerpc/mm/book3s64/pkeys.c | 4 ++++
11 files changed, 38 insertions(+), 18 deletions(-)
@@ -54,7 +54,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,*PPbits._PAGE_USERisalreadyPPbit0x2,soweonly*needtoaddin0x1ifit'saread-onlyuserpage*/-rflags=htab_convert_pte_flags(new_pte);+rflags=htab_convert_pte_flags(new_pte,flags);rpte=__real_pte(__pte(old_pte),ptep,PTRS_PER_PTE);if(cpu_has_feature(CPU_FTR_NOEXECUTE)&&
@@ -72,7 +72,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,*Handlethesubpageprotectionbits*/subpg_pte=new_pte&~subpg_prot;-rflags=htab_convert_pte_flags(subpg_pte);+rflags=htab_convert_pte_flags(subpg_pte,flags);if(cpu_has_feature(CPU_FTR_NOEXECUTE)&&!cpu_has_feature(CPU_FTR_COHERENT_ICACHE)){
@@ -260,7 +260,7 @@ int __hash_page_64K(unsigned long ea, unsigned long access,new_pte|=_PAGE_DIRTY;}while(!pte_xchg(ptep,__pte(old_pte),__pte(new_pte)));-rflags=htab_convert_pte_flags(new_pte);+rflags=htab_convert_pte_flags(new_pte,flags);rpte=__real_pte(__pte(old_pte),ptep,PTRS_PER_PTE);if(cpu_has_feature(CPU_FTR_NOEXECUTE)&&
@@ -57,7 +57,7 @@ int __hash_page_thp(unsigned long ea, unsigned long access, unsigned long vsid,if(!(old_pmd&(H_PAGE_THP_HUGE|_PAGE_DEVMAP)))return0;-rflags=htab_convert_pte_flags(new_pmd);+rflags=htab_convert_pte_flags(new_pmd,flags);#if 0if(!cpu_has_feature(CPU_FTR_COHERENT_ICACHE)){
@@ -72,7 +72,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,if(old_pte&(H_PAGE_THP_HUGE|_PAGE_DEVMAP))return0;-rflags=htab_convert_pte_flags(new_pte);+rflags=htab_convert_pte_flags(new_pte,flags);if(unlikely(mmu_psize==MMU_PAGE_16G))offset=PTRS_PER_PUD;else
@@ -240,7 +240,7 @@ unsigned long htab_convert_pte_flags(unsigned long pteflags)*/rflags|=HPTE_R_M;-rflags|=pte_to_hpte_pkey_bits(pteflags);+rflags|=pte_to_hpte_pkey_bits(pteflags,flags);returnrflags;}
@@ -255,7 +255,7 @@ int htab_bolt_mapping(unsigned long vstart, unsigned long vend,shift=mmu_psize_defs[psize].shift;step=1<<shift;-prot=htab_convert_pte_flags(prot);+prot=htab_convert_pte_flags(prot,HPTE_USE_KERNEL_KEY);DBG("htab_bolt_mapping(%lx..%lx -> %lx (%lx,%d,%d)\n",vstart,vend,pstart,prot,psize,ssize);
@@ -1301,12 +1301,14 @@ int hash_page_mm(struct mm_struct *mm, unsigned long ea,vsid=get_kernel_vsid(ea,mmu_kernel_ssize);psize=mmu_vmalloc_psize;ssize=mmu_kernel_ssize;+flags|=HPTE_USE_KERNEL_KEY;break;caseIO_REGION_ID:vsid=get_kernel_vsid(ea,mmu_kernel_ssize);psize=mmu_io_psize;ssize=mmu_kernel_ssize;+flags|=HPTE_USE_KERNEL_KEY;break;default:/*
@@ -1864,7 +1866,7 @@ static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)unsignedlonghash;unsignedlongvsid=get_kernel_vsid(vaddr,mmu_kernel_ssize);unsignedlongvpn=hpt_vpn(vaddr,vsid,mmu_kernel_ssize);-unsignedlongmode=htab_convert_pte_flags(pgprot_val(PAGE_KERNEL));+unsignedlongmode=htab_convert_pte_flags(pgprot_val(PAGE_KERNEL),HPTE_USE_KERNEL_KEY);longret;hash=hpt_hash(vpn,PAGE_SHIFT,mmu_kernel_ssize);
@@ -190,6 +190,10 @@ void __init pkey_early_init_devtree(void)reserved_allocation_mask|=(0x1<<1);default_uamor&=~(0x3ul<<pkeyshift(1));+/* handle key 3 which is used by kernel for KAUP */+reserved_allocation_mask|=(0x1<<3);+default_uamor&=~(0x3ul<<pkeyshift(3));+/**PreventtheusageofOSreservedkeys.UpdateUAMOR*forthosekeys.Alsomarktherestofthebitsinthe
In later patches during exec, we would like to access default regs.kuap to
control access to the user mapping. Having thread.regs set early makes the
code changes simpler.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/thread_info.h | 2 --
arch/powerpc/kernel/process.c | 37 +++++++++++++++++---------
2 files changed, 25 insertions(+), 14 deletions(-)
@@ -77,10 +77,8 @@ struct thread_info {/* how to get the thread information struct from C */externintarch_dup_task_struct(structtask_struct*dst,structtask_struct*src);-#ifdef CONFIG_PPC_BOOK3S_64voidarch_setup_new_exec(void);#define arch_setup_new_exec arch_setup_new_exec-#endif#endif /* __ASSEMBLY__ */
This prepare kernel to operate with a different value than userspace AMR.
For this, AMR needs to be saved and restored on entry and return from the
kernel.
With KUAP we modify kernel AMR when accessing user address from the kernel
via copy_to/from_user interfaces.
If MMU_FTR_KEY is enabled we always use the key mechanism to implement KUAP
feature. If MMU_FTR_KEY is not supported and if we support MMU_FTR_KUAP
(radix translation on POWER9), we can skip restoring AMR on return
to userspace. Userspace won't be using AMR in that specific config.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 141 ++++++++++++++++++-----
arch/powerpc/kernel/entry_64.S | 6 +-
arch/powerpc/kernel/exceptions-64s.S | 4 +-
arch/powerpc/kernel/syscall_64.c | 26 ++++-
4 files changed, 144 insertions(+), 33 deletions(-)
@@ -13,18 +13,47 @@#ifdef __ASSEMBLY__-.macrokuap_restore_amrgpr1,gpr2-#ifdef CONFIG_PPC_KUAP+.macrokuap_restore_user_amrgpr1+#if defined(CONFIG_PPC_MEM_KEYS)BEGIN_MMU_FTR_SECTION_NESTED(67)-mfspr\gpr1,SPRN_AMR+/*+*AMRisgoingtobedifferentwhen+*returningtouserspace.+*/+ld\gpr1,STACK_REGS_KUAP(r1)+isync+mtsprSPRN_AMR,\gpr1++/* No isync required, see kuap_restore_user_amr() */+END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY,67)+#endif+.endm++.macrokuap_restore_kernel_amrgpr1,gpr2+#if defined(CONFIG_PPC_MEM_KEYS)+BEGIN_MMU_FTR_SECTION_NESTED(67)+b99f// handle_pkey_restore_amr+END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY,67)++BEGIN_MMU_FTR_SECTION_NESTED(68)+b99f// handle_kuap_restore_amr+MMU_FTR_SECTION_ELSE_NESTED(68)+b100f// skip_restore_amr+ALT_MMU_FTR_SECTION_END_NESTED_IFSET(MMU_FTR_KUAP,68)++99:+/*+*AMRisgoingtobemostlythesamesinceweare+*returningtothekernel.Compareanddoamtspr.+*/ld\gpr2,STACK_REGS_KUAP(r1)+mfspr\gpr1,SPRN_AMRcmpd\gpr1,\gpr2-beq998f+beq100fisyncmtsprSPRN_AMR,\gpr2/* No isync required, see kuap_restore_amr() */-998:-END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_KUAP,67)+100:// skip_restore_amr#endif.endm
@@ -40,23 +69,58 @@#endif.endm+/*+*MMU_FTR_PKEYandMMU_FTR_KUAPcanbothbeenabledonaplatform.Weprefer+*PKEYoverKUAPifbothcanbeenabledontheplatform.+*+*WithKUAPonlyenabledonexceptionifwearecomingfromuserspacewedon't+*savetheAMRatall,becausetheexpectationisthatuserspacecan'tchange+*theAMRifKUAPfeatureisenabled.+*/.macrokuap_save_amr_and_lockgpr1,gpr2,use_cr,msr_pr_cr-#ifdef CONFIG_PPC_KUAP+#if defined(CONFIG_PPC_MEM_KEYS)+BEGIN_MMU_FTR_SECTION_NESTED(67)+b101f// handle_pkey_save_amr+END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY,67)++BEGIN_MMU_FTR_SECTION_NESTED(68)+b99f// handle_kuap_save_amr+MMU_FTR_SECTION_ELSE_NESTED(68)+b100f// skip_save_amr+ALT_MMU_FTR_SECTION_END_NESTED_IFSET(MMU_FTR_KUAP,68)+++99:// handle_kuap_save_amr.ifnb\msr_pr_cr-bne\msr_pr_cr,99f+/*+*WeavoidchangingAMRoutsidethekernel+*henceskipthiscompletely.+*/+bne\msr_pr_cr,100f// from userspace.endif++101:// handle_pkey_save_amrmfspr\gpr1,SPRN_AMRstd\gpr1,STACK_REGS_KUAP(r1)-li\gpr2,(AMR_KUAP_BLOCKED>>AMR_KUAP_SHIFT)-sldi\gpr2,\gpr2,AMR_KUAP_SHIFT++/*+*updatekernelAMRwithAMR_KUAP_BLOCKEDonly+*ifKUAPfeatureisenabled+*/+BEGIN_MMU_FTR_SECTION_NESTED(69)+LOAD_REG_IMMEDIATE(\gpr2,AMR_KUAP_BLOCKED)cmpd\use_cr,\gpr1,\gpr2-beq\use_cr,99f-// We don't isync here because we very recently entered via rfid+beq\use_cr,102f+/*+*Wedon'tisyncherebecauseweveryrecentlyenteredviaaninterrupt+*/mtsprSPRN_AMR,\gpr2isync-99:-END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_KUAP,67)+102:+END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_KUAP,69)++100:// skip_save_amr#endif.endm
@@ -35,7 +35,21 @@ notrace long system_call_exception(long r3, long r4, long r5,BUG_ON(!FULL_REGS(regs));BUG_ON(regs->softe!=IRQS_ENABLED);-kuap_check_amr();+#ifdef CONFIG_PPC_MEM_KEYS+if(mmu_has_feature(MMU_FTR_PKEY)){+unsignedlongamr;+/*+*WhenenteringfromuserspacewemostlyhavetheAMR+*differentfromkerneldefaultvalues.Hencedon'tcompare.+*/+amr=mfspr(SPRN_AMR);+regs->kuap=amr;+if(mmu_has_feature(MMU_FTR_KUAP))+mtspr(SPRN_AMR,AMR_KUAP_BLOCKED);+isync();+}else+#endif+kuap_check_amr();account_cpu_user_entry();
@@ -222,6 +236,10 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,account_cpu_user_exit();+/*+*WedothisattheendsothatwedocontextswitchwithKERNELAMR+*/+kuap_restore_user_amr(regs);returnret;}
From: Nicholas Piggin <npiggin@gmail.com> Date: 2020-07-03 09:21:08
Excerpts from Aneesh Kumar K.V's message of June 15, 2020 4:14 pm:
quoted hunk
This prepare kernel to operate with a different value than userspace AMR.
For this, AMR needs to be saved and restored on entry and return from the
kernel.
With KUAP we modify kernel AMR when accessing user address from the kernel
via copy_to/from_user interfaces.
If MMU_FTR_KEY is enabled we always use the key mechanism to implement KUAP
feature. If MMU_FTR_KEY is not supported and if we support MMU_FTR_KUAP
(radix translation on POWER9), we can skip restoring AMR on return
to userspace. Userspace won't be using AMR in that specific config.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 141 ++++++++++++++++++-----
arch/powerpc/kernel/entry_64.S | 6 +-
arch/powerpc/kernel/exceptions-64s.S | 4 +-
arch/powerpc/kernel/syscall_64.c | 26 ++++-
4 files changed, 144 insertions(+), 33 deletions(-)
@@ -13,18 +13,47 @@#ifdef __ASSEMBLY__-.macrokuap_restore_amrgpr1,gpr2-#ifdef CONFIG_PPC_KUAP+.macrokuap_restore_user_amrgpr1+#if defined(CONFIG_PPC_MEM_KEYS)BEGIN_MMU_FTR_SECTION_NESTED(67)-mfspr\gpr1,SPRN_AMR+/*+*AMRisgoingtobedifferentwhen+*returningtouserspace.+*/+ld\gpr1,STACK_REGS_KUAP(r1)+isync+mtsprSPRN_AMR,\gpr1++/* No isync required, see kuap_restore_user_amr() */+END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY,67)+#endif+.endm++.macrokuap_restore_kernel_amrgpr1,gpr2+#if defined(CONFIG_PPC_MEM_KEYS)+BEGIN_MMU_FTR_SECTION_NESTED(67)+b99f// handle_pkey_restore_amr+END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY,67)++BEGIN_MMU_FTR_SECTION_NESTED(68)+b99f// handle_kuap_restore_amr+MMU_FTR_SECTION_ELSE_NESTED(68)+b100f// skip_restore_amr+ALT_MMU_FTR_SECTION_END_NESTED_IFSET(MMU_FTR_KUAP,68)++99:+/*+*AMRisgoingtobemostlythesamesinceweare+*returningtothekernel.Compareanddoamtspr.+*/ld\gpr2,STACK_REGS_KUAP(r1)+mfspr\gpr1,SPRN_AMRcmpd\gpr1,\gpr2-beq998f+beq100fisyncmtsprSPRN_AMR,\gpr2/* No isync required, see kuap_restore_amr() */-998:-END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_KUAP,67)+100:// skip_restore_amr
Can't you code it like this? (_IFCLR requires none of the bits to be
set)
BEGIN_MMU_FTR_SECTION_NESTED(67)
b 99f // nothing using AMR, no need to restore
END_MMU_FTR_SECTION_NESTED_IFCLR(MMU_FTR_PKEY | MMU_FTR_KUAP, 67)
That saves you a branch in the common case of using AMR. Similar
for others.
quoted hunk
@@ -69,22 +133,40 @@ extern u64 default_uamor;-static inline void kuap_restore_amr(struct pt_regs *regs, unsigned long amr)+static inline void kuap_restore_user_amr(struct pt_regs *regs) {- if (mmu_has_feature(MMU_FTR_KUAP) && unlikely(regs->kuap != amr)) {- isync();- mtspr(SPRN_AMR, regs->kuap);- /*- * No isync required here because we are about to RFI back to- * previous context before any user accesses would be made,- * which is a CSI.- */+ if (!mmu_has_feature(MMU_FTR_PKEY))+ return;
If you have PKEY but not KUAP, do you still have to restore?
+
+ isync();
+ mtspr(SPRN_AMR, regs->kuap);
+ /*
+ * No isync required here because we are about to rfi
+ * back to previous context before any user accesses
+ * would be made, which is a CSI.
+ */
+}
+
+static inline void kuap_restore_kernel_amr(struct pt_regs *regs,
+ unsigned long amr)
+{
+ if (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_PKEY)) {
+
+ if (unlikely(regs->kuap != amr)) {
+ isync();
+ mtspr(SPRN_AMR, regs->kuap);
+ /*
+ * No isync required here because we are about to rfi
+ * back to previous context before any user accesses
+ * would be made, which is a CSI.
+ */
+ }
}
}
static inline unsigned long kuap_get_and_check_amr(void)
{
- if (mmu_has_feature(MMU_FTR_KUAP)) {
+ if (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_PKEY)) {
unsigned long amr = mfspr(SPRN_AMR);
if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG)) /* kuap_check_amr() */
WARN_ON_ONCE(amr != AMR_KUAP_BLOCKED);
We could do a static key that's based on this condition, but that can
wait for another day.
@@ -35,7 +35,21 @@ notrace long system_call_exception(long r3, long r4, long r5,BUG_ON(!FULL_REGS(regs));BUG_ON(regs->softe!=IRQS_ENABLED);-kuap_check_amr();+#ifdef CONFIG_PPC_MEM_KEYS+if(mmu_has_feature(MMU_FTR_PKEY)){+unsignedlongamr;+/*+*WhenenteringfromuserspacewemostlyhavetheAMR+*differentfromkerneldefaultvalues.Hencedon'tcompare.+*/+amr=mfspr(SPRN_AMR);+regs->kuap=amr;+if(mmu_has_feature(MMU_FTR_KUAP))+mtspr(SPRN_AMR,AMR_KUAP_BLOCKED);+isync();
isync should be inside the if(). Again do pkeys need to save this if
KUAP is not being used? I haven't really looked at how all that works,
but what's changing for the PKEY && !KUAP case?
This would be nice if it could all go into a wrapper function rather
than ifdef.
@@ -222,6 +236,10 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3, account_cpu_user_exit();+ /*+ * We do this at the end so that we do context switch with KERNEL AMR+ */+ kuap_restore_user_amr(regs); return ret;
Comment doesn't make sense, newline required before return.
quoted hunk
}
@@ -306,6 +324,10 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned account_cpu_user_exit();+ /*+ * We do this at the end so that we do context switch with KERNEL AMR+ */+ kuap_restore_user_amr(regs);
Duplicated comments I prefer to just have like this instead of trying to
keep them in sync. Can complete the circular reference by having a
* similarly in interrupt_exit_user_prepare
in the main comment, but if they come close to one another in the same
file it's not so important to keep them together.
+ kuap_restore_user_amr(regs); /* see syscall_exit_prepare */
quoted hunk
return ret;
}
@@ -376,7 +398,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign * which would cause Read-After-Write stalls. Hence, we take the AMR * value from the check above. */- kuap_restore_amr(regs, amr);+ kuap_restore_kernel_amr(regs, amr); return ret; }
Excerpts from Aneesh Kumar K.V's message of June 15, 2020 4:14 pm:
quoted
This prepare kernel to operate with a different value than userspace AMR.
For this, AMR needs to be saved and restored on entry and return from the
kernel.
With KUAP we modify kernel AMR when accessing user address from the kernel
via copy_to/from_user interfaces.
If MMU_FTR_KEY is enabled we always use the key mechanism to implement KUAP
feature. If MMU_FTR_KEY is not supported and if we support MMU_FTR_KUAP
(radix translation on POWER9), we can skip restoring AMR on return
to userspace. Userspace won't be using AMR in that specific config.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 141 ++++++++++++++++++-----
arch/powerpc/kernel/entry_64.S | 6 +-
arch/powerpc/kernel/exceptions-64s.S | 4 +-
arch/powerpc/kernel/syscall_64.c | 26 ++++-
4 files changed, 144 insertions(+), 33 deletions(-)
@@ -13,18 +13,47 @@#ifdef __ASSEMBLY__-.macrokuap_restore_amrgpr1,gpr2-#ifdef CONFIG_PPC_KUAP+.macrokuap_restore_user_amrgpr1+#if defined(CONFIG_PPC_MEM_KEYS)BEGIN_MMU_FTR_SECTION_NESTED(67)-mfspr\gpr1,SPRN_AMR+/*+*AMRisgoingtobedifferentwhen+*returningtouserspace.+*/+ld\gpr1,STACK_REGS_KUAP(r1)+isync+mtsprSPRN_AMR,\gpr1++/* No isync required, see kuap_restore_user_amr() */+END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY,67)+#endif+.endm++.macrokuap_restore_kernel_amrgpr1,gpr2+#if defined(CONFIG_PPC_MEM_KEYS)+BEGIN_MMU_FTR_SECTION_NESTED(67)+b99f// handle_pkey_restore_amr+END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY,67)++BEGIN_MMU_FTR_SECTION_NESTED(68)+b99f// handle_kuap_restore_amr+MMU_FTR_SECTION_ELSE_NESTED(68)+b100f// skip_restore_amr+ALT_MMU_FTR_SECTION_END_NESTED_IFSET(MMU_FTR_KUAP,68)++99:+/*+*AMRisgoingtobemostlythesamesinceweare+*returningtothekernel.Compareanddoamtspr.+*/ld\gpr2,STACK_REGS_KUAP(r1)+mfspr\gpr1,SPRN_AMRcmpd\gpr1,\gpr2-beq998f+beq100fisyncmtsprSPRN_AMR,\gpr2/* No isync required, see kuap_restore_amr() */-998:-END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_KUAP,67)+100:// skip_restore_amr
Can't you code it like this? (_IFCLR requires none of the bits to be
set)
BEGIN_MMU_FTR_SECTION_NESTED(67)
b 99f // nothing using AMR, no need to restore
END_MMU_FTR_SECTION_NESTED_IFCLR(MMU_FTR_PKEY | MMU_FTR_KUAP, 67)
That saves you a branch in the common case of using AMR. Similar
for others.
Yes i could switch to that. The code is taking extra 200 cycles even
with KUAP/KUEP disabled and no keys being used on hash. I am yet to
analyze this closely. So will rework things based on that analysis.
quoted
@@ -69,22 +133,40 @@ extern u64 default_uamor;-static inline void kuap_restore_amr(struct pt_regs *regs, unsigned long amr)+static inline void kuap_restore_user_amr(struct pt_regs *regs) {- if (mmu_has_feature(MMU_FTR_KUAP) && unlikely(regs->kuap != amr)) {- isync();- mtspr(SPRN_AMR, regs->kuap);- /*- * No isync required here because we are about to RFI back to- * previous context before any user accesses would be made,- * which is a CSI.- */+ if (!mmu_has_feature(MMU_FTR_PKEY))+ return;
If you have PKEY but not KUAP, do you still have to restore?
Yes, because user space pkey is now set on the exit path. This is needed
to handle things like exec/fork().
quoted
+
+ isync();
+ mtspr(SPRN_AMR, regs->kuap);
+ /*
+ * No isync required here because we are about to rfi
+ * back to previous context before any user accesses
+ * would be made, which is a CSI.
+ */
+}
+
+static inline void kuap_restore_kernel_amr(struct pt_regs *regs,
+ unsigned long amr)
+{
+ if (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_PKEY)) {
+
+ if (unlikely(regs->kuap != amr)) {
+ isync();
+ mtspr(SPRN_AMR, regs->kuap);
+ /*
+ * No isync required here because we are about to rfi
+ * back to previous context before any user accesses
+ * would be made, which is a CSI.
+ */
+ }
}
}
static inline unsigned long kuap_get_and_check_amr(void)
{
- if (mmu_has_feature(MMU_FTR_KUAP)) {
+ if (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_PKEY)) {
unsigned long amr = mfspr(SPRN_AMR);
if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG)) /* kuap_check_amr() */
WARN_ON_ONCE(amr != AMR_KUAP_BLOCKED);
We could do a static key that's based on this condition, but that can
wait for another day.
@@ -35,7 +35,21 @@ notrace long system_call_exception(long r3, long r4, long r5,BUG_ON(!FULL_REGS(regs));BUG_ON(regs->softe!=IRQS_ENABLED);-kuap_check_amr();+#ifdef CONFIG_PPC_MEM_KEYS+if(mmu_has_feature(MMU_FTR_PKEY)){+unsignedlongamr;+/*+*WhenenteringfromuserspacewemostlyhavetheAMR+*differentfromkerneldefaultvalues.Hencedon'tcompare.+*/+amr=mfspr(SPRN_AMR);+regs->kuap=amr;+if(mmu_has_feature(MMU_FTR_KUAP))+mtspr(SPRN_AMR,AMR_KUAP_BLOCKED);+isync();
isync should be inside the if(). Again do pkeys need to save this if
KUAP is not being used? I haven't really looked at how all that works,
but what's changing for the PKEY && !KUAP case?
There is no SPR switch in context switch now and all the AMR/IAMR
handling is now in the exit to userspace.
This would be nice if it could all go into a wrapper function rather
than ifdef.
@@ -222,6 +236,10 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3, account_cpu_user_exit();+ /*+ * We do this at the end so that we do context switch with KERNEL AMR+ */+ kuap_restore_user_amr(regs); return ret;
Comment doesn't make sense, newline required before return.
Ok the detail there was we need to make sure we restore AMR towrads the
end and make sure all the kernel code continue to run with KERNEL AMR
value. There is a schedule() call in there with _TIF_NEED_RESCHED. But
those details are not really relevant. That was me tracking down some
issues and writing comment around that part of the code. The only real
detail is switch to userspace AMR in the end.
quoted
}
@@ -306,6 +324,10 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned account_cpu_user_exit();+ /*+ * We do this at the end so that we do context switch with KERNEL AMR+ */+ kuap_restore_user_amr(regs);
Duplicated comments I prefer to just have like this instead of trying to
keep them in sync. Can complete the circular reference by having a
* similarly in interrupt_exit_user_prepare
in the main comment, but if they come close to one another in the same
file it's not so important to keep them together.
+ kuap_restore_user_amr(regs); /* see syscall_exit_prepare */
quoted
return ret;
}
@@ -376,7 +398,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign * which would cause Read-After-Write stalls. Hence, we take the AMR * value from the check above. */- kuap_restore_amr(regs, amr);+ kuap_restore_kernel_amr(regs, amr); return ret; }
From: Nicholas Piggin <npiggin@gmail.com> Date: 2020-07-07 06:25:47
Excerpts from Aneesh Kumar K.V's message of July 3, 2020 7:30 pm:
On 7/3/20 2:48 PM, Nicholas Piggin wrote:
quoted
Excerpts from Aneesh Kumar K.V's message of June 15, 2020 4:14 pm:
quoted
This prepare kernel to operate with a different value than userspace AMR.
For this, AMR needs to be saved and restored on entry and return from the
kernel.
With KUAP we modify kernel AMR when accessing user address from the kernel
via copy_to/from_user interfaces.
If MMU_FTR_KEY is enabled we always use the key mechanism to implement KUAP
feature. If MMU_FTR_KEY is not supported and if we support MMU_FTR_KUAP
(radix translation on POWER9), we can skip restoring AMR on return
to userspace. Userspace won't be using AMR in that specific config.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 141 ++++++++++++++++++-----
arch/powerpc/kernel/entry_64.S | 6 +-
arch/powerpc/kernel/exceptions-64s.S | 4 +-
arch/powerpc/kernel/syscall_64.c | 26 ++++-
4 files changed, 144 insertions(+), 33 deletions(-)
@@ -13,18 +13,47 @@#ifdef __ASSEMBLY__-.macrokuap_restore_amrgpr1,gpr2-#ifdef CONFIG_PPC_KUAP+.macrokuap_restore_user_amrgpr1+#if defined(CONFIG_PPC_MEM_KEYS)BEGIN_MMU_FTR_SECTION_NESTED(67)-mfspr\gpr1,SPRN_AMR+/*+*AMRisgoingtobedifferentwhen+*returningtouserspace.+*/+ld\gpr1,STACK_REGS_KUAP(r1)+isync+mtsprSPRN_AMR,\gpr1++/* No isync required, see kuap_restore_user_amr() */+END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY,67)+#endif+.endm++.macrokuap_restore_kernel_amrgpr1,gpr2+#if defined(CONFIG_PPC_MEM_KEYS)+BEGIN_MMU_FTR_SECTION_NESTED(67)+b99f// handle_pkey_restore_amr+END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY,67)++BEGIN_MMU_FTR_SECTION_NESTED(68)+b99f// handle_kuap_restore_amr+MMU_FTR_SECTION_ELSE_NESTED(68)+b100f// skip_restore_amr+ALT_MMU_FTR_SECTION_END_NESTED_IFSET(MMU_FTR_KUAP,68)++99:+/*+*AMRisgoingtobemostlythesamesinceweare+*returningtothekernel.Compareanddoamtspr.+*/ld\gpr2,STACK_REGS_KUAP(r1)+mfspr\gpr1,SPRN_AMRcmpd\gpr1,\gpr2-beq998f+beq100fisyncmtsprSPRN_AMR,\gpr2/* No isync required, see kuap_restore_amr() */-998:-END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_KUAP,67)+100:// skip_restore_amr
Can't you code it like this? (_IFCLR requires none of the bits to be
set)
BEGIN_MMU_FTR_SECTION_NESTED(67)
b 99f // nothing using AMR, no need to restore
END_MMU_FTR_SECTION_NESTED_IFCLR(MMU_FTR_PKEY | MMU_FTR_KUAP, 67)
That saves you a branch in the common case of using AMR. Similar
for others.
Yes i could switch to that. The code is taking extra 200 cycles even
with KUAP/KUEP disabled and no keys being used on hash. I am yet to
analyze this closely. So will rework things based on that analysis.
quoted
quoted
@@ -69,22 +133,40 @@ extern u64 default_uamor;-static inline void kuap_restore_amr(struct pt_regs *regs, unsigned long amr)+static inline void kuap_restore_user_amr(struct pt_regs *regs) {- if (mmu_has_feature(MMU_FTR_KUAP) && unlikely(regs->kuap != amr)) {- isync();- mtspr(SPRN_AMR, regs->kuap);- /*- * No isync required here because we are about to RFI back to- * previous context before any user accesses would be made,- * which is a CSI.- */+ if (!mmu_has_feature(MMU_FTR_PKEY))+ return;
If you have PKEY but not KUAP, do you still have to restore?
Yes, because user space pkey is now set on the exit path. This is needed
to handle things like exec/fork().
quoted
quoted
+
+ isync();
+ mtspr(SPRN_AMR, regs->kuap);
+ /*
+ * No isync required here because we are about to rfi
+ * back to previous context before any user accesses
+ * would be made, which is a CSI.
+ */
+}
+
+static inline void kuap_restore_kernel_amr(struct pt_regs *regs,
+ unsigned long amr)
+{
+ if (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_PKEY)) {
+
+ if (unlikely(regs->kuap != amr)) {
+ isync();
+ mtspr(SPRN_AMR, regs->kuap);
+ /*
+ * No isync required here because we are about to rfi
+ * back to previous context before any user accesses
+ * would be made, which is a CSI.
+ */
+ }
}
}
static inline unsigned long kuap_get_and_check_amr(void)
{
- if (mmu_has_feature(MMU_FTR_KUAP)) {
+ if (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_PKEY)) {
unsigned long amr = mfspr(SPRN_AMR);
if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG)) /* kuap_check_amr() */
WARN_ON_ONCE(amr != AMR_KUAP_BLOCKED);
We could do a static key that's based on this condition, but that can
wait for another day.
@@ -35,7 +35,21 @@ notrace long system_call_exception(long r3, long r4, long r5,BUG_ON(!FULL_REGS(regs));BUG_ON(regs->softe!=IRQS_ENABLED);-kuap_check_amr();+#ifdef CONFIG_PPC_MEM_KEYS+if(mmu_has_feature(MMU_FTR_PKEY)){+unsignedlongamr;+/*+*WhenenteringfromuserspacewemostlyhavetheAMR+*differentfromkerneldefaultvalues.Hencedon'tcompare.+*/+amr=mfspr(SPRN_AMR);+regs->kuap=amr;+if(mmu_has_feature(MMU_FTR_KUAP))+mtspr(SPRN_AMR,AMR_KUAP_BLOCKED);+isync();
isync should be inside the if(). Again do pkeys need to save this if
KUAP is not being used? I haven't really looked at how all that works,
but what's changing for the PKEY && !KUAP case?
There is no SPR switch in context switch now and all the AMR/IAMR
handling is now in the exit to userspace.
If we have pkeys and no kuap, we could keep the switch in context
switch?
If you don't think it's worth bothering to optimise that case because we
expect KUAP to be used, that's probably okay although maybe an
adjustment to the comment (we don't expect userspace to have different
from kernel values if kernel is not using it for KUAP).
quoted
quoted
+ /*
+ * We do this at the end so that we do context switch with KERNEL AMR
+ */
+ kuap_restore_user_amr(regs);
return ret;
Comment doesn't make sense, newline required before return.
Ok the detail there was we need to make sure we restore AMR towrads the
end and make sure all the kernel code continue to run with KERNEL AMR
value. There is a schedule() call in there with _TIF_NEED_RESCHED. But
those details are not really relevant. That was me tracking down some
issues and writing comment around that part of the code. The only real
detail is switch to userspace AMR in the end.
Yep, I don't think that comment is needed at all. A space before the
return would be nice. I guess after the account_cpu_user_exit is fine,
that thing's a pain anyway that needs to be changed to avoid an SPR
stall I think so I'll look at that afterward anyway.
Thanks,
Nick
Child thread.kuap value is inherited from the parent in copy_thread_tls. We still
need to make sure when the child returns from a fork in the kernel we start with the kernel
default AMR value.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/kernel/process.c | 9 +++++++++
1 file changed, 9 insertions(+)
@@ -1705,6 +1705,15 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long usp,childregs->ppr=DEFAULT_PPR;p->thread.tidr=0;+#endif+/*+*RunwiththecurrentAMRvalueofthekernel+*/+#if defined(CONFIG_PPC_MEM_KEYS)+if(mmu_has_feature(MMU_FTR_KUAP))+kregs->kuap=AMR_KUAP_BLOCKED;+if(mmu_has_feature(MMU_FTR_KUEP))+kregs->kuep=AMR_KUEP_BLOCKED;#endifkregs->nip=ppc_function_entry(f);return0;
This prepare kernel to operate with a different value than userspace IAMR.
For this, IAMR needs to be saved and restored on entry and return from the
kernel.
If MMU_FTR_KEY is enabled we always use the key mechanism to implement KUEP
feature. If MMU_FTR_KEY is not supported and if we support MMU_FTR_KUEP
(radix translation on POWER9), we can skip restoring IAMR on return
to userspace. Userspace won't be using IAMR in that specific config.
We don't need to save/restore IAMR on reentry into the kernel due to interrupt
because the kernel doesn't modify IAMR internally.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 46 ++++++++++++++++++++++--
arch/powerpc/include/asm/ptrace.h | 6 +++-
arch/powerpc/kernel/asm-offsets.c | 4 +++
arch/powerpc/kernel/syscall_64.c | 8 +++--
4 files changed, 59 insertions(+), 5 deletions(-)
@@ -37,15 +37,19 @@ notrace long system_call_exception(long r3, long r4, long r5,#ifdef CONFIG_PPC_MEM_KEYSif(mmu_has_feature(MMU_FTR_PKEY)){-unsignedlongamr;+unsignedlongamr,iamr;/*-*WhenenteringfromuserspacewemostlyhavetheAMR+*WhenenteringfromuserspacewemostlyhavetheAMR/IAMR*differentfromkerneldefaultvalues.Hencedon'tcompare.*/amr=mfspr(SPRN_AMR);+iamr=mfspr(SPRN_IAMR);regs->kuap=amr;+regs->kuep=iamr;if(mmu_has_feature(MMU_FTR_KUAP))mtspr(SPRN_AMR,AMR_KUAP_BLOCKED);+if(mmu_has_feature(MMU_FTR_KUEP))+mtspr(SPRN_IAMR,AMR_KUEP_BLOCKED);isync();}else#endif
On fork, we inherit from the parent and on exec, we should switch to default_amr values.
Also, avoid changing the AMR register value within the kernel. The kernel now runs with
different AMR values.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 2 ++
arch/powerpc/kernel/process.c | 6 +++++-
arch/powerpc/mm/book3s64/pkeys.c | 18 ++----------------
3 files changed, 9 insertions(+), 17 deletions(-)
@@ -21,8 +21,8 @@ int max_pkey; /* Maximum key value supported */*/u32reserved_allocation_mask;staticu32initial_allocation_mask;/* Bits set for the initially allocated keys */-staticu64default_amr;-staticu64default_iamr;+u64default_amr;+u64default_iamr;/* Allow all keys to be modified by default */u64default_uamor=~0x0UL;/*
We will remove thread.amr/iamr/uamor in a later patch
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/kernel/ptrace/ptrace-view.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
@@ -518,8 +529,8 @@ static int pkey_set(struct task_struct *target, const struct user_regset *regsetreturnret;/* UAMOR determines which bits of the AMR can be set from userspace. */-target->thread.amr=(new_amr&target->thread.uamor)|-(target->thread.amr&~target->thread.uamor);+target->thread.regs->kuap=(new_amr&default_uamor)|+(target->thread.regs->kuap&~default_uamor);return0;}
We don't use CPU_FTR_PKEY anymore. Remove the feature bit and mark it
free.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/cputable.h | 13 ++++++-------
arch/powerpc/kernel/dt_cpu_ftrs.c | 6 ------
2 files changed, 6 insertions(+), 13 deletions(-)
@@ -315,20 +292,21 @@ static bool is_pkey_enabled(int pkey)return!!(uamor_pkey_bits);}+/* FIXME!! what happens to other threads AMR value? */staticinlinevoidinit_amr(intpkey,u8init_bits){u64new_amr_bits=(((u64)init_bits&0x3UL)<<pkeyshift(pkey));-u64old_amr=read_amr()&~((u64)(0x3ul)<<pkeyshift(pkey));+u64old_amr=current_thread_amr()&~((u64)(0x3ul)<<pkeyshift(pkey));-write_amr(old_amr|new_amr_bits);+update_current_thread_amr(old_amr|new_amr_bits);}staticinlinevoidinit_iamr(intpkey,u8init_bits){u64new_iamr_bits=(((u64)init_bits&0x1UL)<<pkeyshift(pkey));-u64old_iamr=read_iamr()&~((u64)(0x1ul)<<pkeyshift(pkey));+u64old_iamr=current_thread_iamr()&~((u64)(0x1ul)<<pkeyshift(pkey));-write_iamr(old_iamr|new_iamr_bits);+update_current_thread_iamr(old_iamr|new_iamr_bits);}/*
@@ -361,33 +339,6 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,return0;}-voidthread_pkey_regs_save(structthread_struct*thread)-{-if(!mmu_has_feature(MMU_FTR_PKEY))-return;--/*-*TODO:Skipsavingregistersif@threadhasn'tusedanykeysyet.-*/-thread->amr=read_amr();-thread->iamr=read_iamr();-thread->uamor=read_uamor();-}--voidthread_pkey_regs_restore(structthread_struct*new_thread,-structthread_struct*old_thread)-{-if(!mmu_has_feature(MMU_FTR_PKEY))-return;--if(old_thread->amr!=new_thread->amr)-write_amr(new_thread->amr);-if(old_thread->iamr!=new_thread->iamr)-write_iamr(new_thread->iamr);-if(old_thread->uamor!=new_thread->uamor)-write_uamor(new_thread->uamor);-}-intexecute_only_pkey(structmm_struct*mm){if(static_branch_likely(&execute_pkey_disabled))
@@ -441,10 +392,10 @@ static bool pkey_access_permitted(int pkey, bool write, bool execute)returntrue;pkey_shift=pkeyshift(pkey);-if(execute&&!(read_iamr()&(IAMR_EX_BIT<<pkey_shift)))+if(execute&&!(current_thread_iamr()&(IAMR_EX_BIT<<pkey_shift)))returntrue;-amr=read_amr();/* Delay reading amr until absolutely needed */+amr=current_thread_amr();return((!write&&!(amr&(AMR_RD_BIT<<pkey_shift)))||(write&&!(amr&(AMR_WR_BIT<<pkey_shift))));}
If an application has configured address protection such that read/write is
denied using pkey even the kernel should receive a FAULT on accessing the same.
This patch use user AMR value stored in pt_regs.kuap to achieve the same.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
@@ -302,14 +302,20 @@ static inline void set_kuap(unsigned long value)static__always_inlinevoidallow_user_access(void__user*to,constvoid__user*from,unsignedlongsize,unsignedlongdir){+unsignedlongthread_amr=0;+// This is written so we can resolve to a single case at build timeBUILD_BUG_ON(!__builtin_constant_p(dir));++if(mmu_has_feature(MMU_FTR_PKEY))+thread_amr=current_thread_amr();+if(dir==KUAP_READ)-set_kuap(AMR_KUAP_BLOCK_WRITE);+set_kuap(thread_amr|AMR_KUAP_BLOCK_WRITE);elseif(dir==KUAP_WRITE)-set_kuap(AMR_KUAP_BLOCK_READ);+set_kuap(thread_amr|AMR_KUAP_BLOCK_READ);elseif(dir==KUAP_READ_WRITE)-set_kuap(0);+set_kuap(thread_amr);elseBUILD_BUG();}
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 <redacted>
---
arch/powerpc/include/asm/book3s/32/kup.h | 4 +--
arch/powerpc/include/asm/book3s/64/kup.h | 28 ++++++++++++++++----
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, 30 insertions(+), 12 deletions(-)
@@ -260,7 +260,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))returntrue;// What's left? Kernel fault on user in well defined regions (extable
@@ -75,8 +74,8 @@#ifdef CONFIG_PPC_KUAP_DEBUGBEGIN_MMU_FTR_SECTION_NESTED(67)mfspr\gpr1,SPRN_AMR-li\gpr2,(AMR_KUAP_BLOCKED>>AMR_KUAP_SHIFT)-sldi\gpr2,\gpr2,AMR_KUAP_SHIFT+/* Prevent access to userspace using any key values */+LOAD_REG_IMMEDIATE(\gpr2,AMR_KUAP_BLOCKED)999:tdne\gpr1,\gpr2EMIT_BUG_ENTRY999b,__FILE__,__LINE__,(BUGFLAG_WARNING|BUGFLAG_ONCE)END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_KUAP,67)
Rename variable to indicate that they are invalid values which we will use to
test ptrace update of pkeys.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
.../selftests/powerpc/ptrace/ptrace-pkey.c | 26 +++++++++----------
1 file changed, 13 insertions(+), 13 deletions(-)
@@ -44,7 +44,7 @@ struct shared_info {unsignedlongamr2;/* AMR value that ptrace should refuse to write to the child. */-unsignedlongamr3;+unsignedlonginvalid_amr;/* IAMR value the parent expects to read from the child. */unsignedlongexpected_iamr;
@@ -100,7 +100,7 @@ static int child(struct shared_info *info)info->amr1|=3ul<<pkeyshift(pkey1);info->amr2|=3ul<<pkeyshift(pkey2);-info->amr3|=info->amr2|3ul<<pkeyshift(pkey3);+info->invalid_amr|=info->amr2|3ul<<pkeyshift(pkey3);if(disable_execute)info->expected_iamr|=1ul<<pkeyshift(pkey1);
@@ -111,8 +111,8 @@ static int child(struct shared_info *info)info->expected_uamor|=3ul<<pkeyshift(pkey1)|3ul<<pkeyshift(pkey2);-info->new_iamr|=1ul<<pkeyshift(pkey1)|1ul<<pkeyshift(pkey2);-info->new_uamor|=3ul<<pkeyshift(pkey1);+info->invalid_iamr|=1ul<<pkeyshift(pkey1)|1ul<<pkeyshift(pkey2);+info->invalid_uamor|=3ul<<pkeyshift(pkey1);/**Wewon'tusepkey3.Wejustwantaplausiblebutinvalidkeytotest
@@ -196,9 +196,9 @@ static int parent(struct shared_info *info, pid_t pid)PARENT_SKIP_IF_UNSUPPORTED(ret,&info->child_sync);PARENT_FAIL_IF(ret,&info->child_sync);-info->amr1=info->amr2=info->amr3=regs[0];-info->expected_iamr=info->new_iamr=regs[1];-info->expected_uamor=info->new_uamor=regs[2];+info->amr1=info->amr2=info->invalid_amr=regs[0];+info->expected_iamr=info->invalid_iamr=regs[1];+info->expected_uamor=info->invalid_uamor=regs[2];/* Wake up child so that it can set itself up. */ret=prod_child(&info->child_sync);
@@ -234,10 +234,10 @@ static int parent(struct shared_info *info, pid_t pid)returnret;/* Write invalid AMR value in child. */-ret=ptrace_write_regs(pid,NT_PPC_PKEY,&info->amr3,1);+ret=ptrace_write_regs(pid,NT_PPC_PKEY,&info->invalid_amr,1);PARENT_FAIL_IF(ret,&info->child_sync);-printf("%-30s AMR: %016lx\n",ptrace_write_running,info->amr3);+printf("%-30s AMR: %016lx\n",ptrace_write_running,info->invalid_amr);/* Wake up child so that it can verify it didn't change. */ret=prod_child(&info->child_sync);
@@ -249,7 +249,7 @@ static int parent(struct shared_info *info, pid_t pid)/* Try to write to IAMR. */regs[0]=info->amr1;-regs[1]=info->new_iamr;+regs[1]=info->invalid_iamr;ret=ptrace_write_regs(pid,NT_PPC_PKEY,regs,2);PARENT_FAIL_IF(!ret,&info->child_sync);
@@ -257,7 +257,7 @@ static int parent(struct shared_info *info, pid_t pid)ptrace_write_running,regs[0],regs[1]);/* Try to write to IAMR and UAMOR. */-regs[2]=info->new_uamor;+regs[2]=info->invalid_uamor;ret=ptrace_write_regs(pid,NT_PPC_PKEY,regs,3);PARENT_FAIL_IF(!ret,&info->child_sync);
@@ -66,11 +66,6 @@ static int sys_pkey_alloc(unsigned long flags, unsigned long init_access_rights)returnsyscall(__NR_pkey_alloc,flags,init_access_rights);}-staticintsys_pkey_free(intpkey)-{-returnsyscall(__NR_pkey_free,pkey);-}-staticintchild(structshared_info*info){unsignedlongreg;
@@ -100,7 +95,11 @@ static int child(struct shared_info *info)info->amr1|=3ul<<pkeyshift(pkey1);info->amr2|=3ul<<pkeyshift(pkey2);-info->invalid_amr|=info->amr2|3ul<<pkeyshift(pkey3);+/*+*invalidamrvaluewherewetrytoforcewrite+*thingswhicharedeinedbyauamorsetting.+*/+info->invalid_amr=info->amr2|(~0x0UL&~info->expected_uamor);if(disable_execute)info->expected_iamr|=1ul<<pkeyshift(pkey1);
@@ -196,9 +190,9 @@ static int parent(struct shared_info *info, pid_t pid)PARENT_SKIP_IF_UNSUPPORTED(ret,&info->child_sync);PARENT_FAIL_IF(ret,&info->child_sync);-info->amr1=info->amr2=info->invalid_amr=regs[0];-info->expected_iamr=info->invalid_iamr=regs[1];-info->expected_uamor=info->invalid_uamor=regs[2];+info->amr1=info->amr2=regs[0];+info->expected_iamr=regs[1];+info->expected_uamor=regs[2];/* Wake up child so that it can set itself up. */ret=prod_child(&info->child_sync);
We can kexec into a kernel that doesn't use memory keys for kernel
mapping (such as an older kernel which doesn't support kuap/kuep with hash
translation). We need to make sure we reset the AMR/IAMR value on kexec
otherwise, the new kernel will use key 0 for kernel mapping and the old
AMR value prevents access to key 0.
This patch also removes reset if IAMR and AMOR in kexec_sequence. Reset of AMOR
is not needed and the IAMR reset is partial (it doesn't do the reset
on secondary cpus) and is redundant with this patch.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 20 ++++++++++++++++++++
arch/powerpc/include/asm/kup.h | 14 ++++++++++++++
arch/powerpc/kernel/misc_64.S | 14 --------------
arch/powerpc/kexec/core_64.c | 3 +++
arch/powerpc/mm/book3s64/pgtable.c | 3 +++
5 files changed, 40 insertions(+), 14 deletions(-)
@@ -365,6 +365,26 @@ static inline bool bad_kuap_fault(struct pt_regs *regs, unsigned long address,}#endif /* CONFIG_PPC_KUAP */+#define reset_kuap reset_kuap+staticinlinevoidreset_kuap(void)+{+if(mmu_has_feature(MMU_FTR_KUAP)){+mtspr(SPRN_AMR,0);+/* Do we need isync()? We are going via a kexec reset */+isync();+}+}++#define reset_kuep reset_kuep+staticinlinevoidreset_kuep(void)+{+if(mmu_has_feature(MMU_FTR_KUEP)){+mtspr(SPRN_IAMR,0);+/* Do we need isync()? We are going via a kexec reset */+isync();+}+}+#endif /* __ASSEMBLY__ */#endif /* _ASM_POWERPC_BOOK3S_64_KUP_H */
@@ -413,20 +413,6 @@ _GLOBAL(kexec_sequence)lir0,0stdr0,16(r1)-BEGIN_FTR_SECTION-/*-*ThisisthebesttimetoturnAMR/IAMRoff.-*key0isusedinradixforsupervisor<->user-*protection,butonhashkey0isreserved-*ideallywewanttoenterwithacleanstate.-*NOTE,werelyonr0being0fromabove.-*/-mtsprSPRN_IAMR,r0-BEGIN_FTR_SECTION_NESTED(42)-mtsprSPRN_AMOR,r0-END_FTR_SECTION_NESTED_IFSET(CPU_FTR_HVMODE,42)-END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)-/*saveregsforlocalvarsonnewstack.*yes,wewon't go back, but ...*/
The next set of patches adds support for kuap with hash translation.
In preparation for that rename/move kuap related functions to
non radix names.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 18 +++++++++---------
arch/powerpc/include/asm/mmu.h | 6 +++---
arch/powerpc/mm/book3s64/pkeys.c | 2 +-
3 files changed, 13 insertions(+), 13 deletions(-)
@@ -24,7 +24,7 @@mtsprSPRN_AMR,\gpr2/* No isync required, see kuap_restore_amr() */998:-END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_RADIX_KUAP,67)+END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_KUAP,67)#endif.endm
Make KUAP/KUEP key a variable and also check whether the platform
limit the max key such that we can't use the key for KUAP/KEUP.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
.../powerpc/include/asm/book3s/64/hash-pkey.h | 22 +--------
arch/powerpc/include/asm/book3s/64/kup.h | 1 +
arch/powerpc/mm/book3s64/pkeys.c | 46 +++++++++++++++++--
3 files changed, 43 insertions(+), 26 deletions(-)
@@ -170,6 +174,18 @@ void __init pkey_early_init_devtree(void)default_uamor&=~(0x3ul<<pkeyshift(execute_only_key));}+if(unlikely(max_pkey<=kup_key)){+/*+*Insufficientnumberofkeystosupport+*KUAP/KUEPfeature.+*/+kup_key=-1;+}else{+/* handle key which is used by kernel for KAUP */+reserved_allocation_mask|=(0x1<<kup_key);+default_uamor&=~(0x3ul<<pkeyshift(kup_key));+}+/**Allowaccessforonlykey0.Andpreventanyothermodification.*/
@@ -190,9 +206,6 @@ void __init pkey_early_init_devtree(void)reserved_allocation_mask|=(0x1<<1);default_uamor&=~(0x3ul<<pkeyshift(1));-/* handle key 3 which is used by kernel for KAUP */-reserved_allocation_mask|=(0x1<<3);-default_uamor&=~(0x3ul<<pkeyshift(3));/**PreventtheusageofOSreservedkeys.UpdateUAMOR
Both IAMR and uamor are privileged and cannot be updated by userspace. Hence
we also don't allow ptrace interface to update them. Don't update them in the
test. Also expected_iamr is only changed if we can allocate a DISABLE_EXECUTE
pkey.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)