linux-kernel - Re: [PATCH] arm64/mm: Fix mapping_dirty

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a44e78dc-0761-cdf4-fbf9-f2395b40aa55@vmware.com>
Date:   Wed, 7 Apr 2021 13:00:36 -0400
From:   Zack Rusin <zackr@...are.com>
To:     Catalin Marinas <catalin.marinas@....com>
Cc:     linux-kernel@...r.kernel.org, Will Deacon <will@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] arm64/mm: Fix mapping_dirty_helpers with arm64

On 4/2/21 7:26 AM, Catalin Marinas wrote:
> On Wed, Mar 31, 2021 at 02:23:44PM -0400, Zack Rusin wrote:
>> The pagetable walk callbacks in mm/mapping_dirty_helpers.c depend
>> on a set of helpers from which pud_dirty(pud) was missing. I'm
>> assuming mapping_dirty_helpers weren't used on ARM64 before
>> because the missing pud_dirty is causing a compilation error.
>>
>> The drivers/gpu/drm/vmwgfx code uses mapping_dirty_helpers and
>> has been ported to ARM64 but it depends on this code getting in
>> first in order to compile/work.
>>
>> Cc: Catalin Marinas <catalin.marinas@....com>
>> Cc: Will Deacon <will@...nel.org>
>> Cc: Peter Zijlstra <peterz@...radead.org>
>> Cc: linux-arm-kernel@...ts.infradead.org
>> Signed-off-by: Zack Rusin <zackr@...are.com>
>> ---
>>   arch/arm64/include/asm/pgtable.h | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index 47027796c2f9..ecd80f87a996 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -451,6 +451,7 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
>>   #define pfn_pmd(pfn,prot)	__pmd(__phys_to_pmd_val((phys_addr_t)(pfn) << PAGE_SHIFT) | pgprot_val(prot))
>>   #define mk_pmd(page,prot)	pfn_pmd(page_to_pfn(page),prot)
>>   
>> +#define pud_dirty(pud)		pte_dirty(pud_pte(pud))
>>   #define pud_young(pud)		pte_young(pud_pte(pud))
>>   #define pud_mkyoung(pud)	pte_pud(pte_mkyoung(pud_pte(pud)))
>>   #define pud_write(pud)		pte_write(pud_pte(pud))
> 
> I think pud_dirty() should only be defined if
> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD which is not the case on
> arm64. Well, the pud_young/mkyoung/write should probably also be removed
> until we have pud pages on arm64.

Agreed. I got confused by pud_young and co being defined here even 
though they're all shielded by CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD 
when they're used and incorrectly assumed the practice was to define 
them for arm64 here whether CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD 
was defined or not.

I'll go ahead and shield the pud_dirty with 
CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD in mapping_dirty_helpers.c 
instead.

z