On 16.05.2018 16:22, Janosch Frank wrote:
> On 16.05.2018 16:13, David Hildenbrand wrote:
>> On 16.05.2018 11:10, Janosch Frank wrote:
>>> From: Janosch Frank <frankja@xxxxxxxxxxxxxxxxxx>
>>>
>>> Tests no-execute (Instruction Execution Protection) DAT protection.
>>>
>>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>>> ---
>>>
>>> This was originally part of my first patchset, but was pushed back
>>> after David introduced virtual memory allocation. It's more a RFC, as
>>> I'm not completely happy with the memory management manipulation I do.
>>>
>>> ---
>>> lib/s390x/asm/interrupt.h | 1 +
>>> lib/s390x/interrupt.c | 9 +++++
>>> s390x/Makefile | 1 +
>>> s390x/iep.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++
>>> s390x/unittests.cfg | 4 +++
>>> 5 files changed, 104 insertions(+)
>>> create mode 100644 s390x/iep.c
>>>
>>> diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
>>> index 3ccc8e3..39ae534 100644
>>> --- a/lib/s390x/asm/interrupt.h
>>> +++ b/lib/s390x/asm/interrupt.h
>>> @@ -15,6 +15,7 @@ void handle_pgm_int(void);
>>> void expect_pgm_int(void);
>>> uint16_t clear_pgm_int(void);
>>> void check_pgm_int_code(uint16_t code);
>>> +void register_pgm_handler(void (*handler)(struct lowcore *));
>>>
>>> /* Activate low-address protection */
>>> static inline void low_prot_enable(void)
>>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>>> index 56c7603..fe6cdba 100644
>>> --- a/lib/s390x/interrupt.c
>>> +++ b/lib/s390x/interrupt.c
>>> @@ -15,6 +15,7 @@
>>>
>>> static bool pgm_int_expected;
>>> static struct lowcore *lc;
>>> +static void (*custom_pgm_handler)(struct lowcore *);
>>>
>>> void expect_pgm_int(void)
>>> {
>>> @@ -41,8 +42,16 @@ void check_pgm_int_code(uint16_t code)
>>> code == lc->pgm_int_code, code, lc->pgm_int_code);
>>> }
>>>
>>> +void register_pgm_handler(void (*handler)(struct lowcore *))
>>> +{
>>> + custom_pgm_handler = handler;
>>> +}
>>> +
>>> static void fixup_pgm_int(void)
>>> {
>>> + if (custom_pgm_handler)
>>> + return custom_pgm_handler(lc);
>>> +
>>> switch (lc->pgm_int_code) {
>>> case PGM_INT_CODE_PRIVILEGED_OPERATION:
>>> /* Normal operation is in supervisor state, so this exception
>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>> index abc3242..d4275a1 100644
>>> --- a/s390x/Makefile
>>> +++ b/s390x/Makefile
>>> @@ -9,6 +9,7 @@ tests += $(TEST_DIR)/pfmf.elf
>>> tests += $(TEST_DIR)/cmm.elf
>>> tests += $(TEST_DIR)/vector.elf
>>> tests += $(TEST_DIR)/gs.elf
>>> +tests += $(TEST_DIR)/iep.elf
>>>
>>> all: directories test_cases
>>>
>>> diff --git a/s390x/iep.c b/s390x/iep.c
>>> new file mode 100644
>>> index 0000000..87f3007
>>> --- /dev/null
>>> +++ b/s390x/iep.c
>>> @@ -0,0 +1,89 @@
>>> +/*
>>> + * Instruction Execution Prevention (IEP) DAT test.
>>> + *
>>> + * Copyright (c) 2018 IBM Corp
>>> + *
>>> + * Authors:
>>> + * Janosch Frank <frankja@xxxxxxxxxxxxx>
>>> + *
>>> + * This code is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU Library General Public License version 2.
>>> + */
>>> +#include <libcflat.h>
>>> +#include <vmalloc.h>
>>> +#include <asm/facility.h>
>>> +#include <asm/interrupt.h>
>>> +#include <asm/page.h>
>>> +#include <asm/pgtable.h>
>>> +#include <asm-generic/barrier.h>
>>> +
>>> +static pteval_t *get_pte(pgd_t *pgtable, uintptr_t vaddr)
>>> +{
>>> + pgd_t *pgd = pgd_offset(pgtable, vaddr);
>>> + p4d_t *p4d = p4d_offset(pgd, vaddr);
>>> + pud_t *pud = pud_offset(p4d, vaddr);
>>> + pmd_t *pmd = pmd_offset(pud, vaddr);
>>> + pte_t *pte = pte_offset(pmd, vaddr);
>>> +
>>> + return &pte_val(*pte);
>>> +}
>>
>> You could factor that out into some sort of
>>
>> protect_range() / protect_page()
>> unprotect_range() / unprotect_page()
>
> Would the page protection be enough for now?
> Then I'd add that to our mmu.c
Sure, we can later add protect_segment ... to keep it simple.
>
>>
>> And implement iep handling directly in the core. The handling of
>> resetting the old psw seems to be generic enough.
>
> It will break two tests, that's why I added a custom handler.
We should be able to easily check which type of protection it was, no?
(IEP vs. DAT vs. lowprot)
Or what exactly makes these fail?
>
>>
>> The all you would have to do is enable iep and check for
>> PGM_INT_CODE_PROTECTION
>>
>>> +
>>> +void iep_handler(struct lowcore *lc)
>>> +{
>>> + if (lc->pgm_int_code == PGM_INT_CODE_PROTECTION)
>>> + lc->pgm_old_psw.addr = lc->sw_int_grs[14];
>>> +}
>>> +
>>> +static void test_iep(void)
>>> +{
>>> + uint16_t *code;
>>> + uint8_t *iepbuf = 0;
>>> + void (*fn)(void);
>>> + pteval_t *pte;
>>> + pgd_t *pgtable = (pgd_t *)(stctg(1) & ~(ASCE_DT_REGION1 | REGION_TABLE_LENGTH));
>>> +
>>> +
>>> + /* Enable IEP */
>>> + ctl_set_bit(0, 20);
>>> +
>>> + /* Get and protect a page with the IEP bit */
>>> + iepbuf = alloc_page();
>>> + pte = get_pte(pgtable, (uintptr_t)iepbuf);
>>> + ipte((uintptr_t)iepbuf, pte);
>>> + *pte |= PAGE_ENTRY_IEP;
>>> + *pte &= ~PAGE_ENTRY_I;
>>> + mb();
>>> +
>>> + register_pgm_handler(&iep_handler);
>>> +
>>> + /* Code branches into r14 which contains the return address. */
>>> + code = (uint16_t *)iepbuf;
>>> + *code = 0x07fe;
>>> + asm volatile("" : : "m" (code));
>>> + fn = (void *)code;
>>> + mb();
>>> +
>>> + expect_pgm_int();
>>> + /* Jump into protected page */
>>> + fn();
>>> + check_pgm_int_code(PGM_INT_CODE_PROTECTION);
>>> + ctl_clear_bit(0, 20);
>>> +}
>>> +
>>> +int main(void)
>>> +{
>>> + bool has_iep = test_facility(130);
>>> +
>>> + report_prefix_push("iep");
>>> + report_xfail("DAT IEP available", !has_iep, has_iep);
>>> + if (!has_iep)
>>> + goto done;
>>> +
>>> + /* Setup DAT 1:1 mapping and memory management */
>>> + setup_vm();
>>> + test_iep();
>>> +
>>> +done:
>>> + report_prefix_pop();
>>> + return report_summary();
>>> +}
>>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>>> index ff7eea1..760402e 100644
>>> --- a/s390x/unittests.cfg
>>> +++ b/s390x/unittests.cfg
>>> @@ -55,3 +55,7 @@ file = vector.elf
>>>
>>> [gs]
>>> file = gs.elf
>>> +
>>> +[iep]
>>> +file = iep.elf
>>> +
>>>
>>
>>
>
>
--
Thanks,
David / dhildenb
