On Mon, Nov 06, 2017 at 12:57:15AM -0800, Ram Pai wrote:
> PAPR defines 'ibm,processor-storage-keys' property. It exports two
> values. The first value holds the number of data-access keys and the
> second holds the number of instruction-access keys. Due to a bug in
> the firmware, instruction-access keys is always reported as zero.
> However any key can be configured to disable data-access and/or disable
> execution-access. The inavailablity of the second value is not a
> big handicap, though it could have been used to determine if the
> platform supported disable-execution-access.
>
> Non PAPR platforms do not define this property in the device tree yet.
> Here, we hardcode CPUs that support pkey by consulting
> PowerISA3.0
>
> This patch calculates the number of keys supported by the platform.
> Alsi it determines the platform support for read/write/execution access
> support for pkeys.
>
> Signed-off-by: Ram Pai <linuxram(a)us.ibm.com>
> ---
>
....snip...
> +static inline bool pkey_mmu_enabled(void)
> +{
> + if (firmware_has_feature(FW_FEATURE_LPAR))
> + return pkeys_total;
> + else
> + return cpu_has_feature(CPU_FTR_PKEY);
> +}
> +
> void __init pkey_initialize(void)
> {
> int os_reserved, i;
> @@ -46,14 +54,9 @@ void __init pkey_initialize(void)
> __builtin_popcountl(ARCH_VM_PKEY_FLAGS >> VM_PKEY_SHIFT)
> != (sizeof(u64) * BITS_PER_BYTE));
>
> - /*
> - * Disable the pkey system till everything is in place. A subsequent
> - * patch will enable it.
> - */
> - static_branch_enable(&pkey_disabled);
> -
> - /* Lets assume 32 keys */
> - pkeys_total = 32;
vvvvvvvvvvvvvvvvvvvv
> + /* Let's assume 32 keys if we are not told the number of pkeys. */
> + if (!pkeys_total)
> + pkeys_total = 32;
^^^^^^^^^^^^^^^^^^^^
There is a small bug here.
On a KVM guest or a LPAR, if the device tree
does not expose pkeys, the pkey-subsystem must be disabled.
Unfortunately, the code above blindly sets the pkeys_total to 32.
This confuses pkey_mmu_enabled() into returning true. Because of this
bug the guest errorneously enables pkey-subsystem.
The fix is to delete the code marked above.
>
> /*
> * Adjust the upper limit, based on the number of bits supported by
> @@ -62,11 +65,19 @@ void __init pkey_initialize(void)
> pkeys_total = min_t(int, pkeys_total,
> (ARCH_VM_PKEY_FLAGS >> VM_PKEY_SHIFT));
>
> + if (!pkey_mmu_enabled() || radix_enabled() || !pkeys_total)
> + static_branch_enable(&pkey_disabled);
> + else
> + static_branch_disable(&pkey_disabled);
> +
RP
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi
Le 06/11/2017 à 09:56, Ram Pai a écrit :
> Memory protection keys enable applications to protect its
> address space from inadvertent access from or corruption
> by itself.
>
> These patches along with the pte-bit freeing patch series
> enables the protection key feature on powerpc; 4k and 64k
> hashpage kernels. It also changes the generic and x86
> code to expose memkey features through sysfs. Finally
> testcases and Documentation is updated.
>
> All patches can be found at --
> https://github.com/rampai/memorykeys.git memkey.v9
As far as I can see you are focussing the implementation on 64 bits
powerpc. This could also be implemented on 32 bits powerpc, for instance
the 8xx has MMU Access Protection Registers which can be used to define
16 domains and could I think be used for implementing protection keys.
Of course the challenge after that would be to find 4 spare PTE bits,
I'm sure we can find them on the 8xx, at least when using 16k pages we
have 2 bits already available, then by merging PAGE_SHARED and PAGE_USER
and by reducing PAGE_RO to only one bit we can get the 4 spare bits.
Therefore I think it would be great if you could implement a framework
common to both PPC32 and PPC64.
Christophe
>
> The overall idea:
> -----------------
> A process allocates a key and associates it with
> an address range within its address space.
> The process then can dynamically set read/write
> permissions on the key without involving the
> kernel. Any code that violates the permissions
> of the address space; as defined by its associated
> key, will receive a segmentation fault.
>
> This patch series enables the feature on PPC64 HPTE
> platform.
>
> ISA3.0 section 5.7.13 describes the detailed
> specifications.
>
>
> Highlevel view of the design:
> ---------------------------
> When an application associates a key with a address
> address range, program the key in the Linux PTE.
> When the MMU detects a page fault, allocate a hash
> page and program the key into HPTE. And finally
> when the MMU detects a key violation; due to
> invalid application access, invoke the registered
> signal handler and provide the violated key number.
>
>
> Testing:
> -------
> This patch series has passed all the protection key
> tests available in the selftest directory.The
> tests are updated to work on both x86 and powerpc.
> The selftests have passed on x86 and powerpc hardware.
>
> History:
> -------
> version v9:
> (1) used jump-labels to optimize code
> -- Balbir
> (2) fixed a register initialization bug noted
> by Balbir
> (3) fixed inappropriate use of paca to pass
> siginfo and keys to signal handler
> (4) Cleanup of comment style not to be right
> justified -- mpe
> (5) restructured the patches to depend on the
> availability of VM_PKEY_BIT4 in
> include/linux/mm.h
> (6) Incorporated comments from Dave Hansen
> towards changes to selftest and got
> them tested on x86.
>
> version v8:
> (1) Contents of the AMR register withdrawn from
> the siginfo structure. Applications can always
> read the AMR register.
> (2) AMR/IAMR/UAMOR are now available through
> ptrace system call. -- thanks to Thiago
> (3) code changes to handle legacy power cpus
> that do not support execute-disable.
> (4) incorporates many code improvement
> suggestions.
>
> version v7:
> (1) refers to device tree property to enable
> protection keys.
> (2) adds 4K PTE support.
> (3) fixes a couple of bugs noticed by Thiago
> (4) decouples this patch series from arch-
> independent code. This patch series can
> now stand by itself, with one kludge
> patch(2).
> version v7:
> (1) refers to device tree property to enable
> protection keys.
> (2) adds 4K PTE support.
> (3) fixes a couple of bugs noticed by Thiago
> (4) decouples this patch series from arch-
> independent code. This patch series can
> now stand by itself, with one kludge
> patch(2).
>
> version v6:
> (1) selftest changes are broken down into 20
> incremental patches.
> (2) A separate key allocation mask that
> includes PKEY_DISABLE_EXECUTE is
> added for powerpc
> (3) pkey feature is enabled for 64K HPT case
> only. RPT and 4k HPT is disabled.
> (4) Documentation is updated to better
> capture the semantics.
> (5) introduced arch_pkeys_enabled() to find
> if an arch enables pkeys. Correspond-
> ing change the logic that displays
> key value in smaps.
> (6) code rearranged in many places based on
> comments from Dave Hansen, Balbir,
> Anshuman.
> (7) fixed one bug where a bogus key could be
> associated successfully in
> pkey_mprotect().
>
> version v5:
> (1) reverted back to the old design -- store
> the key in the pte, instead of bypassing
> it. The v4 design slowed down the hash
> page path.
> (2) detects key violation when kernel is told
> to access user pages.
> (3) further refined the patches into smaller
> consumable units
> (4) page faults handlers captures the fault-
> ing key
> from the pte instead of the vma. This
> closes a race between where the key
> update in the vma and a key fault caused
> by the key programmed in the pte.
> (5) a key created with access-denied should
> also set it up to deny write. Fixed it.
> (6) protection-key number is displayed in
> smaps the x86 way.
>
> version v4:
> (1) patches no more depend on the pte bits
> to program the hpte
> -- comment by Balbir
> (2) documentation updates
> (3) fixed a bug in the selftest.
> (4) unlike x86, powerpc lets signal handler
> change key permission bits; the
> change will persist across signal
> handler boundaries. Earlier we
> allowed the signal handler to
> modify a field in the siginfo
> structure which would than be used
> by the kernel to program the key
> protection register (AMR)
> -- resolves a issue raised by Ben.
> "Calls to sys_swapcontext with a
> made-up context will end up with a
> crap AMR if done by code who didn't
> know about that register".
> (5) these changes enable protection keys on
> 4k-page kernel aswell.
>
> version v3:
> (1) split the patches into smaller consumable
> patches.
> (2) added the ability to disable execute
> permission on a key at creation.
> (3) rename calc_pte_to_hpte_pkey_bits() to
> pte_to_hpte_pkey_bits()
> -- suggested by Anshuman
> (4) some code optimization and clarity in
> do_page_fault()
> (5) A bug fix while invalidating a hpte slot
> in __hash_page_4K()
> -- noticed by Aneesh
>
>
> version v2:
> (1) documentation and selftest added.
> (2) fixed a bug in 4k hpte backed 64k pte
> where page invalidation was not
> done correctly, and initialization
> of second-part-of-the-pte was not
> done correctly if the pte was not
> yet Hashed with a hpte.
> -- Reported by Aneesh.
> (3) Fixed ABI breakage caused in siginfo
> structure.
> -- Reported by Anshuman.
>
>
> version v1: Initial version
>
> Ram Pai (47):
> mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS
> is enabled
> mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
> powerpc: initial pkey plumbing
> powerpc: track allocation status of all pkeys
> powerpc: helper function to read,write AMR,IAMR,UAMOR registers
> powerpc: helper functions to initialize AMR, IAMR and UAMOR registers
> powerpc: cleanup AMR, IAMR when a key is allocated or freed
> powerpc: implementation for arch_set_user_pkey_access()
> powerpc: ability to create execute-disabled pkeys
> powerpc: store and restore the pkey state across context switches
> powerpc: introduce execute-only pkey
> powerpc: ability to associate pkey to a vma
> powerpc: implementation for arch_override_mprotect_pkey()
> powerpc: map vma key-protection bits to pte key bits.
> powerpc: Program HPTE key protection bits
> powerpc: helper to validate key-access permissions of a pte
> powerpc: check key protection for user page access
> powerpc: implementation for arch_vma_access_permitted()
> powerpc: Handle exceptions caused by pkey violation
> powerpc: introduce get_mm_addr_key() helper
> powerpc: Deliver SEGV signal on pkey violation
> powerpc: Enable pkey subsystem
> powerpc: sys_pkey_alloc() and sys_pkey_free() system calls
> powerpc: sys_pkey_mprotect() system call
> powerpc: add sys_pkey_modify() system call
> mm, x86 : introduce arch_pkeys_enabled()
> mm: display pkey in smaps if arch_pkeys_enabled() is true
> Documentation/x86: Move protecton key documentation to arch neutral
> directory
> Documentation/vm: PowerPC specific updates to memory protection keys
> selftest/x86: Move protecton key selftest to arch neutral directory
> selftest/vm: rename all references to pkru to a generic name
> selftest/vm: move generic definitions to header file
> selftest/vm: typecast the pkey register
> selftest/vm: generic function to handle shadow key register
> selftest/vm: fix the wrong assert in pkey_disable_set()
> selftest/vm: fixed bugs in pkey_disable_clear()
> selftest/vm: clear the bits in shadow reg when a pkey is freed.
> selftest/vm: fix alloc_random_pkey() to make it really random
> selftest/vm: introduce two arch independent abstraction
> selftest/vm: pkey register should match shadow pkey
> selftest/vm: generic cleanup
> selftest/vm: powerpc implementation for generic abstraction
> selftest/vm: fix an assertion in test_pkey_alloc_exhaust()
> selftest/vm: associate key on a mapped page and detect access
> violation
> selftest/vm: associate key on a mapped page and detect write
> violation
> selftest/vm: detect write violation on a mapped access-denied-key
> page
> selftest/vm: sub-page allocator
>
> Thiago Jung Bauermann (4):
> powerpc/ptrace: Add memory protection key regset
> mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface
> selftests/powerpc: Add ptrace tests for Protection Key register
> selftests/powerpc: Add core file test for Protection Key register
>
> Documentation/vm/protection-keys.txt | 161 +++
> Documentation/x86/protection-keys.txt | 85 --
> arch/powerpc/Kconfig | 15 +
> arch/powerpc/include/asm/book3s/64/mmu-hash.h | 5 +
> arch/powerpc/include/asm/book3s/64/mmu.h | 10 +
> arch/powerpc/include/asm/book3s/64/pgtable.h | 42 +-
> arch/powerpc/include/asm/bug.h | 1 +
> arch/powerpc/include/asm/cputable.h | 15 +-
> arch/powerpc/include/asm/mman.h | 13 +-
> arch/powerpc/include/asm/mmu.h | 9 +
> arch/powerpc/include/asm/mmu_context.h | 24 +
> arch/powerpc/include/asm/pkeys.h | 247 ++++
> arch/powerpc/include/asm/processor.h | 5 +
> arch/powerpc/include/asm/systbl.h | 4 +
> arch/powerpc/include/asm/unistd.h | 6 +-
> arch/powerpc/include/uapi/asm/elf.h | 1 +
> arch/powerpc/include/uapi/asm/mman.h | 6 +
> arch/powerpc/include/uapi/asm/unistd.h | 4 +
> arch/powerpc/kernel/entry_64.S | 9 +
> arch/powerpc/kernel/process.c | 7 +
> arch/powerpc/kernel/prom.c | 18 +
> arch/powerpc/kernel/ptrace.c | 66 +
> arch/powerpc/kernel/traps.c | 19 +-
> arch/powerpc/mm/Makefile | 1 +
> arch/powerpc/mm/fault.c | 49 +-
> arch/powerpc/mm/hash_utils_64.c | 29 +
> arch/powerpc/mm/mmu_context_book3s64.c | 2 +
> arch/powerpc/mm/pkeys.c | 463 +++++++
> arch/x86/include/asm/mmu_context.h | 4 +-
> arch/x86/include/asm/pkeys.h | 2 +
> arch/x86/kernel/fpu/xstate.c | 5 +
> arch/x86/kernel/setup.c | 8 -
> arch/x86/mm/pkeys.c | 9 +
> fs/proc/task_mmu.c | 16 +-
> include/linux/mm.h | 12 +-
> include/linux/pkeys.h | 7 +-
> include/uapi/linux/elf.h | 1 +
> mm/mprotect.c | 88 ++
> tools/testing/selftests/powerpc/include/reg.h | 1 +
> tools/testing/selftests/powerpc/ptrace/Makefile | 5 +-
> tools/testing/selftests/powerpc/ptrace/core-pkey.c | 438 ++++++
> .../testing/selftests/powerpc/ptrace/ptrace-pkey.c | 443 ++++++
> tools/testing/selftests/vm/Makefile | 1 +
> tools/testing/selftests/vm/pkey-helpers.h | 405 ++++++
> tools/testing/selftests/vm/protection_keys.c | 1464 ++++++++++++++++++++
> tools/testing/selftests/x86/Makefile | 2 +-
> tools/testing/selftests/x86/pkey-helpers.h | 220 ---
> tools/testing/selftests/x86/protection_keys.c | 1395 -------------------
> 48 files changed, 4095 insertions(+), 1747 deletions(-)
> create mode 100644 Documentation/vm/protection-keys.txt
> delete mode 100644 Documentation/x86/protection-keys.txt
> create mode 100644 arch/powerpc/include/asm/pkeys.h
> create mode 100644 arch/powerpc/mm/pkeys.c
> create mode 100644 tools/testing/selftests/powerpc/ptrace/core-pkey.c
> create mode 100644 tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
> create mode 100644 tools/testing/selftests/vm/pkey-helpers.h
> create mode 100644 tools/testing/selftests/vm/protection_keys.c
> delete mode 100644 tools/testing/selftests/x86/pkey-helpers.h
> delete mode 100644 tools/testing/selftests/x86/protection_keys.c
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ram,
On Mon, Nov 06, 2017 at 12:57:36AM -0800, Ram Pai wrote:
> @@ -206,12 +209,14 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)
>
> trapno = uctxt->uc_mcontext.gregs[REG_TRAPNO];
> ip = uctxt->uc_mcontext.gregs[REG_IP_IDX];
> - fpregset = uctxt->uc_mcontext.fpregs;
> - fpregs = (void *)fpregset;
Since you removed all references for fpregset now, you probably want to
remove the declaration of the variable above.
> @@ -219,20 +224,21 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)
> * state. We just assume that it is here.
> */
> fpregs += 0x70;
> -#endif
> - pkey_reg_offset = pkey_reg_xstate_offset();
With this code, you removed all the reference for variable
pkey_reg_offset, thus, its declaration could be removed also.
> - *(u64 *)pkey_reg_ptr = 0x00000000;
> + dprintf1("si_pkey from siginfo: %lx\n", si_pkey);
> +#if defined(__i386__) || defined(__x86_64__) /* arch */
> + dprintf1("signal pkey_reg from xsave: %016lx\n", *pkey_reg_ptr);
> + *(u64 *)pkey_reg_ptr &= reset_bits(si_pkey, PKEY_DISABLE_ACCESS);
> +#elif __powerpc64__
Since the variable pkey_reg_ptr is only used for Intel code (inside
#ifdefs), you probably want to #ifdef the variable declaration also,
avoid triggering "unused variable" warning on non-Intel machines.
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 06, 2017 at 05:22:18PM -0800, Ram Pai wrote:
> On Mon, Nov 06, 2017 at 10:28:41PM +0100, Florian Weimer wrote:
> > * Ram Pai:
> >
> > > Testing:
> > > -------
> > > This patch series has passed all the protection key
> > > tests available in the selftest directory.The
> > > tests are updated to work on both x86 and powerpc.
> > > The selftests have passed on x86 and powerpc hardware.
> >
....snip....
> > What about siglongjmp from a signal handler?
>
> On powerpc there is some relief. the permissions on a key can be
> modified from anywhere, including from the signal handler, and the
> effect will be immediate. You dont have to wait till the
> signal handler returns for the key permissions to be restore.
>
> also after return from the sigsetjmp();
> possibly caused by siglongjmp(), the program can restore the permission
> on any key.
>
> Atleast that is my theory. Can you give me a testcase; if you have one
> handy.
>
> >
> > <https://sourceware.org/bugzilla/show_bug.cgi?id=22396>
> >
reading through the bug report, you mention that the following
"The application may not be able to save and restore the protection bits
for all keys because the kernel API does not actually specify that the
set of keys is a small, fixed set."
What exact kernel API do you need? This patch set exposes the total
number of keys and max keys, through sysfs.
https://marc.info/?l=linux-kernel&m=150995950219669&w=2
Is this sufficient? or do you need something else?
RP
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html