| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5646347F.1020805@redhat.com>
Date: Fri, 13 Nov 2015 11:05:35 -0800
From: Laura Abbott <labbott@...hat.com>
To: zhong jiang <zhongjiang@...wei.com>
Cc: Laura Abbott <labbott@...oraproject.org>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will.deacon@....com>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Kees Cook <keescook@...omium.org>,
Xishi Qiu <qiuxishi@...wei.com>,
Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCHv2 2/2] arm64: Allow changing of attributes outside of
modules
On 11/12/2015 06:05 PM, zhong jiang wrote:
> On 2015/11/13 0:31, Laura Abbott wrote:
>> On 11/12/2015 03:55 AM, zhong jiang wrote:
>>> On 2015/11/11 9:57, Laura Abbott wrote:
>>>> Currently, the set_memory_* functions that are implemented for arm64
>>>> are restricted to module addresses only. This was mostly done
>>>> because arm64 maps normal zone memory with larger page sizes to
>>>> improve TLB performance. This has the side effect though of making it
>>>> difficult to adjust attributes at the PAGE_SIZE granularity. There are
>>>> an increasing number of use cases related to security where it is
>>>> necessary to change the attributes of kernel memory. Add functionality
>>>> to the page attribute changing code under a Kconfig to let systems
>>>> designers decide if they want to make the trade off of security for TLB
>>>> pressure.
>>>>
>>>> Signed-off-by: Laura Abbott <labbott@...oraproject.org>
>>>> ---
>>>> v2: Re-worked to account for the full range of addresses. Will also just
>>>> update the section blocks instead of splitting if the addresses are aligned
>>>> properly.
>>>> ---
>>>> arch/arm64/Kconfig | 12 ++++
>>>> arch/arm64/mm/mm.h | 3 +
>>>> arch/arm64/mm/mmu.c | 2 +-
>>>> arch/arm64/mm/pageattr.c | 174 +++++++++++++++++++++++++++++++++++++++++------
>>>> 4 files changed, 170 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index 851fe11..46725e8 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -521,6 +521,18 @@ config ARCH_HAS_CACHE_LINE_SIZE
>>>>
>>>> source "mm/Kconfig"
>>>>
>>>> +config DEBUG_CHANGE_PAGEATTR
>>>> + bool "Allow all kernel memory to have attributes changed"
>>>> + default y
>>>> + help
>>>> + If this option is selected, APIs that change page attributes
>>>> + (RW <-> RO, X <-> NX) will be valid for all memory mapped in
>>>> + the kernel space. The trade off is that there may be increased
>>>> + TLB pressure from finer grained page mapping. Turn on this option
>>>> + if security is more important than performance
>>>> +
>>>> + If in doubt, say Y
>>>> +
>>>> config SECCOMP
>>>> bool "Enable seccomp to safely compute untrusted bytecode"
>>>> ---help---
>>>> diff --git a/arch/arm64/mm/mm.h b/arch/arm64/mm/mm.h
>>>> index ef47d99..7b0dcc4 100644
>>>> --- a/arch/arm64/mm/mm.h
>>>> +++ b/arch/arm64/mm/mm.h
>>>> @@ -1,3 +1,6 @@
>>>> extern void __init bootmem_init(void);
>>>>
>>>> void fixup_init(void);
>>>> +
>>>> +void split_pud(pud_t *old_pud, pmd_t *pmd);
>>>> +void split_pmd(pmd_t *pmd, pte_t *pte);
>>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>>>> index 496c3fd..9353e3c 100644
>>>> --- a/arch/arm64/mm/mmu.c
>>>> +++ b/arch/arm64/mm/mmu.c
>>>> @@ -73,7 +73,7 @@ static void __init *early_alloc(unsigned long sz)
>>>> /*
>>>> * remap a PMD into pages
>>>> */
>>>> -static void split_pmd(pmd_t *pmd, pte_t *pte)
>>>> +void split_pmd(pmd_t *pmd, pte_t *pte)
>>>> {
>>>> unsigned long pfn = pmd_pfn(*pmd);
>>>> unsigned long addr = pfn << PAGE_SHIFT;
>>>> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
>>>> index 3571c73..4a95fed 100644
>>>> --- a/arch/arm64/mm/pageattr.c
>>>> +++ b/arch/arm64/mm/pageattr.c
>>>> @@ -15,25 +15,162 @@
>>>> #include <linux/module.h>
>>>> #include <linux/sched.h>
>>>>
>>>> +#include <asm/pgalloc.h>
>>>> #include <asm/pgtable.h>
>>>> #include <asm/tlbflush.h>
>>>>
>>>> -struct page_change_data {
>>>> - pgprot_t set_mask;
>>>> - pgprot_t clear_mask;
>>>> -};
>>>> +#include "mm.h"
>>>>
>>>> -static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
>>>> - void *data)
>>>> +static int update_pte_range(struct mm_struct *mm, pmd_t *pmd,
>>>> + unsigned long addr, unsigned long end,
>>>> + pgprot_t clear, pgprot_t set)
>>>> {
>>>> - struct page_change_data *cdata = data;
>>>> - pte_t pte = *ptep;
>>>> + pte_t *pte;
>>>> + int err = 0;
>>>> +
>>>> + if (pmd_sect(*pmd)) {
>>>> + if (!IS_ENABLED(CONFIG_DEBUG_CHANGE_PAGEATTR)) {
>>>> + err = -EINVAL;
>>>> + goto out;
>>>> + }
>>>> + pte = pte_alloc_one_kernel(&init_mm, addr);
>>>> + if (!pte) {
>>>> + err = -ENOMEM;
>>>> + goto out;
>>>> + }
>>>> + split_pmd(pmd, pte);
>>>> + __pmd_populate(pmd, __pa(pte), PMD_TYPE_TABLE);
>>>> + }
>>>> +
>>>> +
>>>> + pte = pte_offset_kernel(pmd, addr);
>>>> + if (pte_none(*pte)) {
>>>> + err = -EFAULT;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + do {
>>>> + pte_t p = *pte;
>>>> +
>>>> + p = clear_pte_bit(p, clear);
>>>> + p = set_pte_bit(p, set);
>>>> + set_pte(pte, p);
>>>> +
>>>> + } while (pte++, addr += PAGE_SIZE, addr != end);
>>>> +
>>>> +out:
>>>> + return err;
>>>> +}
>>>> +
>>>> +
>>>> +static int update_pmd_range(struct mm_struct *mm, pud_t *pud,
>>>> + unsigned long addr, unsigned long end,
>>>> + pgprot_t clear, pgprot_t set)
>>>> +{
>>>> + pmd_t *pmd;
>>>> + unsigned long next;
>>>> + int err = 0;
>>>> +
>>>> + if (pud_sect(*pud)) {
>>>> + if (!IS_ENABLED(CONFIG_DEBUG_CHANGE_PAGEATTR)) {
>>>> + err = -EINVAL;
>>>> + goto out;
>>>> + }
>>>> + pmd = pmd_alloc_one(&init_mm, addr);
>>>> + if (!pmd) {
>>>> + err = -ENOMEM;
>>>> + goto out;
>>>> + }
>>>> + split_pud(pud, pmd);
>>>> + pud_populate(&init_mm, pud, pmd);
>>>> + }
>>>> +
>>>>
>>>> - pte = clear_pte_bit(pte, cdata->clear_mask);
>>>> - pte = set_pte_bit(pte, cdata->set_mask);
>>>> + pmd = pmd_offset(pud, addr);
>>>> + if (pmd_none(*pmd)) {
>>>> + err = -EFAULT;
>>>> + goto out;
>>>> + }
>>>> +
>>>
>>> we try to preserve the section area, but the addr | end does not ensure that
>>> physical memory is alignment. In addtion, if numpages cross section area, and
>>> addr points to the physical memory is alignment to the section. In this case,
>>> we should consider to retain the section.
>>>
>>
>> I'm not sure what physical memory you are referring to here. The mapping is
>> already set up so if there is a section mapping we know the physical memory
>> is going to be set up to be a section size. We aren't setting up a new mapping
>> for the physical address so there is no need to check that again. The only
>> way to get the physical address would be to read it out of the section
>> entry which wouldn't give any more information.
>>
>> I'm also not sure what you are referring to with numpages crossing a section
>> area. In update_pud_range and update_pmd_range there are checks if a
>> section can be used. If it can, it updates. The split action is only called
>> if it isn't aligned. The loop ensures this will happen across all possible
>> sections.
>>
>> Thanks,
>> Laura
>>
>>
>
> Hi Laura
>
> In pmd_update_range, Is the pmd pointing to large page if addr is alignment ?
> I mean that whether it need to add pmd_sect() to guarantee.
>
Okay, now I see what you are referring to. Yes, I think you are correct there.
I'll take a look at that for the next revision.
Thanks,
Laura
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists