| 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: <cd80e950-1185-bc12-f844-295b854d429a@arm.com>
Date: Mon, 23 Mar 2020 18:56:47 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: Christophe Leroy <christophe.leroy@....fr>
Cc: linuxppc-dev@...ts.ozlabs.org,
Andrew Morton <akpm@...ux-foundation.org>,
Palmer Dabbelt <palmer@...belt.com>,
linux-kernel@...r.kernel.org, Vineet Gupta <vgupta@...opsys.com>,
linux-arm-kernel@...ts.infradead.org,
Thomas Gleixner <tglx@...utronix.de>,
"Kirill A . Shutemov" <kirill@...temov.name>,
Paul Walmsley <paul.walmsley@...ive.com>,
Borislav Petkov <bp@...en8.de>,
Vasily Gorbik <gor@...ux.ibm.com>,
linux-snps-arc@...ts.infradead.org,
Catalin Marinas <catalin.marinas@....com>,
Ingo Molnar <mingo@...hat.com>,
Christian Borntraeger <borntraeger@...ibm.com>,
Mike Rapoport <rppt@...ux.ibm.com>, x86@...nel.org,
linux-s390@...r.kernel.org, linux-arch@...r.kernel.org,
Will Deacon <will@...nel.org>, linux-riscv@...ts.infradead.org,
"H. Peter Anvin" <hpa@...or.com>,
Paul Mackerras <paulus@...ba.org>,
Heiko Carstens <heiko.carstens@...ibm.com>, linux-mm@...ck.org
Subject: Re: [PATCH] mm/debug: Add tests validating arch page table helpers
for core features
On 03/03/2020 02:54 AM, Christophe Leroy wrote:
> Anshuman Khandual <anshuman.khandual@....com> a écrit :
>
>> On 02/27/2020 04:59 PM, Christophe Leroy wrote:
>>>
>>>
>>> Le 27/02/2020 à 11:33, Anshuman Khandual a écrit :
>>>> This adds new tests validating arch page table helpers for these following
>>>> core memory features. These tests create and test specific mapping types at
>>>> various page table levels.
>>>>
>>>> * SPECIAL mapping
>>>> * PROTNONE mapping
>>>> * DEVMAP mapping
>>>> * SOFTDIRTY mapping
>>>> * SWAP mapping
>>>> * MIGRATION mapping
>>>> * HUGETLB mapping
>>>> * THP mapping
>>>>
>>>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>>>> Cc: Mike Rapoport <rppt@...ux.ibm.com>
>>>> Cc: Vineet Gupta <vgupta@...opsys.com>
>>>> Cc: Catalin Marinas <catalin.marinas@....com>
>>>> Cc: Will Deacon <will@...nel.org>
>>>> Cc: Benjamin Herrenschmidt <benh@...nel.crashing.org>
>>>> Cc: Paul Mackerras <paulus@...ba.org>
>>>> Cc: Michael Ellerman <mpe@...erman.id.au>
>>>> Cc: Heiko Carstens <heiko.carstens@...ibm.com>
>>>> Cc: Vasily Gorbik <gor@...ux.ibm.com>
>>>> Cc: Christian Borntraeger <borntraeger@...ibm.com>
>>>> Cc: Thomas Gleixner <tglx@...utronix.de>
>>>> Cc: Ingo Molnar <mingo@...hat.com>
>>>> Cc: Borislav Petkov <bp@...en8.de>
>>>> Cc: "H. Peter Anvin" <hpa@...or.com>
>>>> Cc: Kirill A. Shutemov <kirill@...temov.name>
>>>> Cc: Paul Walmsley <paul.walmsley@...ive.com>
>>>> Cc: Palmer Dabbelt <palmer@...belt.com>
>>>> Cc: linux-snps-arc@...ts.infradead.org
>>>> Cc: linux-arm-kernel@...ts.infradead.org
>>>> Cc: linuxppc-dev@...ts.ozlabs.org
>>>> Cc: linux-s390@...r.kernel.org
>>>> Cc: linux-riscv@...ts.infradead.org
>>>> Cc: x86@...nel.org
>>>> Cc: linux-arch@...r.kernel.org
>>>> Cc: linux-kernel@...r.kernel.org
>>>> Suggested-by: Catalin Marinas <catalin.marinas@....com>
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@....com>
>>>> ---
>>>> Tested on arm64 and x86 platforms without any test failures. But this has
>>>> only been built tested on several other platforms. Individual tests need
>>>> to be verified on all current enabling platforms for the test i.e s390,
>>>> ppc32, arc etc.
>>>>
>>>> This patch must be applied on v5.6-rc3 after these patches
>>>>
>>>> 1. https://patchwork.kernel.org/patch/11385057/
>>>> 2. https://patchwork.kernel.org/patch/11407715/
>>>>
>>>> OR
>>>>
>>>> This patch must be applied on linux-next (next-20200227) after this patch
>>>>
>>>> 2. https://patchwork.kernel.org/patch/11407715/
>>>>
>>>> mm/debug_vm_pgtable.c | 310 +++++++++++++++++++++++++++++++++++++++++-
>>>> 1 file changed, 309 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>>>> index 96dd7d574cef..3fb90d5b604e 100644
>>>> --- a/mm/debug_vm_pgtable.c
>>>> +++ b/mm/debug_vm_pgtable.c
>>>> @@ -41,6 +41,44 @@
>>>> * wrprotect(entry) = A write protected and not a write entry
>>>> * pxx_bad(entry) = A mapped and non-table entry
>>>> * pxx_same(entry1, entry2) = Both entries hold the exact same value
>>>> + *
>>>> + * Specific feature operations
>>>> + *
>>>> + * pte_mkspecial(entry) = Creates a special entry at PTE level
>>>> + * pte_special(entry) = Tests a special entry at PTE level
>>>> + *
>>>> + * pte_protnone(entry) = Tests a no access entry at PTE level
>>>> + * pmd_protnone(entry) = Tests a no access entry at PMD level
>>>> + *
>>>> + * pte_mkdevmap(entry) = Creates a device entry at PTE level
>>>> + * pmd_mkdevmap(entry) = Creates a device entry at PMD level
>>>> + * pud_mkdevmap(entry) = Creates a device entry at PUD level
>>>> + * pte_devmap(entry) = Tests a device entry at PTE level
>>>> + * pmd_devmap(entry) = Tests a device entry at PMD level
>>>> + * pud_devmap(entry) = Tests a device entry at PUD level
>>>> + *
>>>> + * pte_mksoft_dirty(entry) = Creates a soft dirty entry at PTE level
>>>> + * pmd_mksoft_dirty(entry) = Creates a soft dirty entry at PMD level
>>>> + * pte_swp_mksoft_dirty(entry) = Creates a soft dirty swap entry at PTE level
>>>> + * pmd_swp_mksoft_dirty(entry) = Creates a soft dirty swap entry at PMD level
>>>> + * pte_soft_dirty(entry) = Tests a soft dirty entry at PTE level
>>>> + * pmd_soft_dirty(entry) = Tests a soft dirty entry at PMD level
>>>> + * pte_swp_soft_dirty(entry) = Tests a soft dirty swap entry at PTE level
>>>> + * pmd_swp_soft_dirty(entry) = Tests a soft dirty swap entry at PMD level
>>>> + * pte_clear_soft_dirty(entry) = Clears a soft dirty entry at PTE level
>>>> + * pmd_clear_soft_dirty(entry) = Clears a soft dirty entry at PMD level
>>>> + * pte_swp_clear_soft_dirty(entry) = Clears a soft dirty swap entry at PTE level
>>>> + * pmd_swp_clear_soft_dirty(entry) = Clears a soft dirty swap entry at PMD level
>>>> + *
>>>> + * pte_mkhuge(entry) = Creates a HugeTLB entry at given level
>>>> + * pte_huge(entry) = Tests a HugeTLB entry at given level
>>>> + *
>>>> + * pmd_trans_huge(entry) = Tests a trans huge page at PMD level
>>>> + * pud_trans_huge(entry) = Tests a trans huge page at PUD level
>>>> + * pmd_present(entry) = Tests an entry points to memory at PMD level
>>>> + * pud_present(entry) = Tests an entry points to memory at PUD level
>>>> + * pmd_mknotpresent(entry) = Invalidates an PMD entry for MMU
>>>> + * pud_mknotpresent(entry) = Invalidates an PUD entry for MMU
>>>> */
>>>> #define VMFLAGS (VM_READ|VM_WRITE|VM_EXEC)
>>>> @@ -287,6 +325,233 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp,
>>>> WARN_ON(pmd_bad(pmd));
>>>> }
>>>> +#ifdef CONFIG_ARCH_HAS_PTE_SPECIAL
>>>
>>> Can we avoid ifdefs unless necessary ?
>>>
>>> In mm/memory.c I see things like the following, it means pte_special() always exist and a #ifdef is not necessary.
>>
>> True, #ifdef here can be dropped here, done.
>>
>>>
>>> if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL)) {
>>> if (likely(!pte_special(pte)))
>>> goto check_pfn;
>>> if (vma->vm_ops && vma->vm_ops->find_special_page)
>>> return vma->vm_ops->find_special_page(vma, addr);
>>> if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
>>> return NULL;
>>> if (is_zero_pfn(pfn))
>>> return NULL;
>>> if (pte_devmap(pte))
>>> return NULL;
>>>
>>> print_bad_pte(vma, addr, pte, NULL);
>>> return NULL;
>>> }
>>>
>>>> +static void __init pte_special_tests(unsigned long pfn, pgprot_t prot)
>>>> +{
>>>> + pte_t pte = pfn_pte(pfn, prot);
>>>> +
>>>> + WARN_ON(!pte_special(pte_mkspecial(pte)));
>>>> +}
>>>> +#else
>>>> +static void __init pte_special_tests(unsigned long pfn, pgprot_t prot) { }
>>>> +#endif
>>>> +
>>>> +#ifdef CONFIG_NUMA_BALANCING
>>>
>>> Same here, this ifdef shouldn't be necessary because in /include/asm-generic/pgtable.h we have the following, so a if (IS_ENABLED()) should be enough.
>>>
>>> #ifndef CONFIG_NUMA_BALANCING
>>> /*
>>> * Technically a PTE can be PROTNONE even when not doing NUMA balancing but
>>> * the only case the kernel cares is for NUMA balancing and is only ever set
>>> * when the VMA is accessible. For PROT_NONE VMAs, the PTEs are not marked
>>> * _PAGE_PROTNONE so by by default, implement the helper as "always no". It
>>> * is the responsibility of the caller to distinguish between PROT_NONE
>>> * protections and NUMA hinting fault protections.
>>> */
>>> static inline int pte_protnone(pte_t pte)
>>> {
>>> return 0;
>>> }
>>>
>>> static inline int pmd_protnone(pmd_t pmd)
>>> {
>>> return 0;
>>> }
>>> #endif /* CONFIG_NUMA_BALANCING */
>>
>> True, #ifdef here can be dropped, done. There is something I had missed
>> before, pfn_pmd() requires #ifdef CONFIG_TRANSPARENT_HUGEPAGE instead. We
>> need a pmd_t here with given prot. We cannot go via pfn_pte() followed by
>> pte_pmd(), as the later is platform specific and not available in general.
>
> As many things require CONFIG_TRANSPARENT_HUGEPAGE, maybe it would be worth creating an additional C file with the related functions and build it conditionnaly to CONFIG_TRANSPARENT_HUGEPAGE
>
Apologies for the delayed response here. Any split in the test will break it's
monolithic structure which is not desirable. Also lack of an explicit dependency
between HAVE_ARCH_TRANSPARENT_HUGEPAGE and HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
makes it difficult to group together fallback dummy stubs from various THP
related test functions here. I am planning to re-spin this patch sooner with
some more tests while also accommodating other previous comments. Hence, will
probably note down this aspect which can then be discussed further if required.
> Christophe
Powered by blists - more mailing lists