linux-kernel - Re: [PATCH] arm64: mm: fix pmd

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 4 Apr 2022 19:40:24 +0530
From:   Aneesh Kumar K V <aneesh.kumar@...ux.ibm.com>
To:     Muchun Song <songmuchun@...edance.com>,
        Will Deacon <will@...nel.org>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Anshuman Khandual <anshuman.khandual@....com>,
        steven.price@....com, lengxujun2007@....com,
        Arnd Bergmann <arnd@...db.de>,
        LAK <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Muchun Song <smuchun@...il.com>,
        Xiongchun duan <duanxiongchun@...edance.com>,
        Qian Cai <quic_qiancai@...cinc.com>
Subject: Re: [PATCH] arm64: mm: fix pmd_leaf()

On 4/4/22 5:10 PM, Muchun Song wrote:
> On Mon, Apr 4, 2022 at 5:20 PM Will Deacon <will@...nel.org> wrote:
>>
>> On Sun, Apr 03, 2022 at 10:49:28AM +0800, Muchun Song wrote:
>>> The pmd_leaf() is used to test a leaf mapped PMD, however, it misses
>>> the PROT_NONE mapped PMD on arm64.  Fix it.  A real world issue [1]
>>> caused by this was reported by Qian Cai.
>>>
>>> Link: https://patchwork.kernel.org/comment/24798260/ [1]
>>> Fixes: 8aa82df3c123 ("arm64: mm: add p?d_leaf() definitions")
>>> Reported-by: Qian Cai <quic_qiancai@...cinc.com>
>>> Signed-off-by: Muchun Song <songmuchun@...edance.com>
>>> ---
>>>   arch/arm64/include/asm/pgtable.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>>> index 94e147e5456c..09eaae46a19b 100644
>>> --- a/arch/arm64/include/asm/pgtable.h
>>> +++ b/arch/arm64/include/asm/pgtable.h
>>> @@ -535,7 +535,7 @@ extern pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
>>>                                 PMD_TYPE_TABLE)
>>>   #define pmd_sect(pmd)                ((pmd_val(pmd) & PMD_TYPE_MASK) == \
>>>                                 PMD_TYPE_SECT)
>>> -#define pmd_leaf(pmd)                pmd_sect(pmd)
>>> +#define pmd_leaf(pmd)                (pmd_present(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT))
>>>   #define pmd_bad(pmd)         (!pmd_table(pmd))
>>>
>>>   #define pmd_leaf_size(pmd)   (pmd_cont(pmd) ? CONT_PMD_SIZE : PMD_SIZE)
>>
>> A bunch of the users of pmd_leaf() already check pmd_present() -- is it
>> documented that we need to handle this check inside the macro? afaict x86
>> doesn't do this either.


ppc64 also doesn't do a pmd_present check.

>>
> 
> arm64 is different from x86 here. pmd_leaf() could return true for
> the none pmd without pmd_present() check, the check of
> pmd_present() aims to exclude the pmd_none() case.  However,
> it could work properly on x86 without pmd_present() or pmd_none().
> So we don't see pmd_present() or pmd_none() check in pmd_leaf().
> For this reason, I think this check is necessary.
> 
> BTW, there are some users of pmd_leaf() (e.g. apply_to_pmd_range,
> walk_pmd_range, ptdump_pmd_entry) which do not check pmd_present()
> or pmd_none() before the call of pmd_leaf().  So it is also necessary
> to add the check.
> 


I would expect pmd_leaf check to return true, if the said pmd page table 
entry can point to a leaf page table entry which can also be a not 
present page table entry?

-aneesh