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

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]

Message-ID: <CAHk-=wisZfwwJo57BRigT5X_uWs6Jw4K3ezPSwCSMBHSeJTHzg@mail.gmail.com>
Date:   Tue, 17 Sep 2019 16:35:46 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Song Liu <songliubraving@...com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        "the arch/x86 maintainers" <x86@...nel.org>
Subject: Re: [GIT pull] x86/pti for 5.4-rc1

On Tue, Sep 17, 2019 at 4:29 PM Song Liu <songliubraving@...com> wrote:
>
> How about we just do:
>
> diff --git i/arch/x86/mm/pti.c w/arch/x86/mm/pti.c
> index b196524759ec..0437f65250db 100644
> --- i/arch/x86/mm/pti.c
> +++ w/arch/x86/mm/pti.c
> @@ -341,6 +341,7 @@ pti_clone_pgtable(unsigned long start, unsigned long end,
>                 }
>
>                 if (pmd_large(*pmd) || level == PTI_CLONE_PMD) {
> +                       WARN_ON_ONCE(addr & ~PMD_MASK);
>                         target_pmd = pti_user_pagetable_walk_pmd(addr);
>                         if (WARN_ON(!target_pmd))
>                                 return;
>
> So it is a "warn and continue" check just for unaligned PMD address.

The problem there is that the "continue" part can be wrong.

Admittedly it requires a pretty crazy setup: you first hit a
pmd_large() entry, but the *next* pmd is regular, so you start doing
the per-page cloning.

And that per-page cloning will be wrong, because it will start in the
middle of the next pmd, because addr wasn't aligned, and the previous
pmd-only clone did

                        addr += PMD_SIZE;

to go to the next case.

See?

Can this happen right now? I'd certainly hope not. But if we're
hardening this code against odd cases that can't currently happen, it
surely is such an odd case.

            Linus