Re: [GIT pull] x86/pti for 5.4-rc1

[Song Liu]

> On Sep 17, 2019, at 11:13 AM, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> 
> On Mon, Sep 16, 2019 at 6:38 AM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>> 
>>  - Handle unaligned addresses gracefully in pti_clone_pagetable(). Not an
>>    issue with current callers, but a correctness problem. Adds a warning
>>    so any caller which hands in an unaligned address gets pointed out
>>    clearly.
> 
> Hmm. I actually thing this one is incomplete.
> 
> Yes, it does it correctly now for the "those addresses are missing" cases.
> 
> But if somebody passes in an unaligned address, it still does the
> wrong thing for the
> 
>                if (pmd_large(*pmd) || level == PTI_CLONE_PMD) {
> 
> case. No?

I guess we need something like the following?

diff --git i/arch/x86/mm/pti.c w/arch/x86/mm/pti.c
index b196524759ec..7846916c3bcd 100644
--- i/arch/x86/mm/pti.c
+++ w/arch/x86/mm/pti.c
@@ -306,6 +306,8 @@ pti_clone_pgtable(unsigned long start, unsigned long end,
 {
        unsigned long addr;

+       if (WARN_ON_ONCE(start & ~PAGE_MASK))
+               return;
        /*
         * Clone the populated PMDs which cover start to end. These PMD areas
         * can have holes.
@@ -341,6 +343,8 @@ pti_clone_pgtable(unsigned long start, unsigned long end,
                }

                if (pmd_large(*pmd) || level == PTI_CLONE_PMD) {
+                       if (WARN_ON_ONCE(addr & ~PMD_MASK))
+                               return;
                        target_pmd = pti_user_pagetable_walk_pmd(addr);
                        if (WARN_ON(!target_pmd))
                                return;

> 
> I've pulled this, since the change is not _wrong_, but it does look
> incomplete to me. Am I missing something?
> 
> Also, it would have been lovely to have some background on how this
> was even noticed. The link in the commit message goes to the
> development thread, but that one doesn't have the original report from
> Song either.

I didn't really notice any issue. I was debugging an increase in iTLB 
miss rate, which was caused by splitting of kernel text page table, 
and was fixed by Thomas in:

https://lore.kernel.org/lkml/alpine.DEB.2.21.1908282355340.1938@xxxxxxxxxxxxxxxxxxxxxxx/

I mistakenly suspected the issue was caused by the pti code, and 
mistakenly proposed the first patch here. It turned out to be useful, 
but it is not related to the original issue. 

Thanks,
Song