On 16.05.2018 16:45, David Hildenbrand wrote:
> 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?
Testing the right bits helps not jumping where we don't want to be...
Alright, I'll send a v2 after my vacation, thanks for the guidance.
Attachment:
signature.asc
Description: OpenPGP digital signature
