All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Song Liu <songliubraving@fb.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
	the arch/x86 maintainers <x86@kernel.org>
Subject: Re: [GIT pull] x86/pti for 5.4-rc1
Date: Tue, 17 Sep 2019 23:28:38 +0000	[thread overview]
Message-ID: <E4AF8E4C-A307-4AE7-85AA-F579D5BFDBDD@fb.com> (raw)
In-Reply-To: <CAHk-=whbOhAmc3FEhnEkdM9AXFuKM+964r+uzzm_Q9qFaxTC7g@mail.gmail.com>



> On Sep 17, 2019, at 12:01 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> On Tue, Sep 17, 2019 at 11:49 AM Song Liu <songliubraving@fb.com> wrote:
>> 
>> 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;
> 
> I don't think we ever care about the low bits of the address below the
> page mask, so this one probably wouldn't make any difference.
> 
> To match the other cases, I'd make it just a plain
> 
>        WARN_ON_ONCE(start & ~PAGE_MASK));
> 
> and leave it at that. The existing commit added the warning, but then
> just made the code work despite it all.  I'd continue that same logic.
> 
>>                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;
> 
> Again, I think to match the other cases, I'd just do
> 
> -                       addr += PMD_SIZE;
> +                       WARN_ON_ONCE(addr & ~PMD_MASK);
> +                       addr = round_up(addr + 1, PMD_SIZE);
> 
> which now admittedly clones too much, but _does_ clone the requested range.
> 
> But maybe it really doesn't matter, since this condition just
> shouldn't happen in the first place.
> 
> And arguably, the "clone more than requested" issue is true, and maybe
> your "warn and refuse to clone by returning" is the right thing to do.
> 
> So I have very few strong opinions in this area, I just reacted to
> looking at the patch that it didn't seem to cover all the cases.

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. 

Thanks,
Song


  reply	other threads:[~2019-09-17 23:29 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-16 13:30 [GIT pull] irq/core for 5.4-rc1 Thomas Gleixner
2019-09-16 13:30 ` [GIT pull] x86/irq " Thomas Gleixner
2019-09-17 20:15   ` pr-tracker-bot
2019-09-16 13:30 ` [GIT pull] smp/hotplug " Thomas Gleixner
2019-09-17 20:15   ` pr-tracker-bot
2019-09-16 13:30 ` [GIT pull] x86/pti " Thomas Gleixner
2019-09-17 18:13   ` Linus Torvalds
2019-09-17 18:48     ` Song Liu
2019-09-17 19:01       ` Linus Torvalds
2019-09-17 23:28         ` Song Liu [this message]
2019-09-17 23:35           ` Linus Torvalds
2019-09-18 10:40             ` Song Liu
2019-09-25  6:23               ` Ingo Molnar
2019-09-17 20:15   ` pr-tracker-bot
2019-09-16 13:30 ` [GIT pull] timers/core " Thomas Gleixner
2019-09-17 20:15   ` pr-tracker-bot
2019-09-16 13:30 ` [GIT pull] timers/urgent " Thomas Gleixner
2019-09-17 20:15   ` pr-tracker-bot
2019-09-16 13:30 ` [GIT pull] x86/apic " Thomas Gleixner
2019-09-17 20:15   ` pr-tracker-bot
2019-09-17 20:15 ` [GIT pull] irq/core " pr-tracker-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E4AF8E4C-A307-4AE7-85AA-F579D5BFDBDD@fb.com \
    --to=songliubraving@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.