All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Dave Hansen <dave@sr71.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	the arch/x86 maintainers <x86@kernel.org>,
	linux-mm <linux-mm@kvack.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Borislav Petkov <bp@alien8.de>, Andi Kleen <ak@linux.intel.com>,
	Michal Hocko <mhocko@suse.com>,
	Dave Hansen <dave.hansen@linux.intel.com>
Subject: Re: [PATCH 6/6] x86: Fix stray A/D bit setting into non-present PTEs
Date: Fri, 01 Jul 2016 13:12:55 -0500	[thread overview]
Message-ID: <878txlb520.fsf@x220.int.ebiederm.org> (raw)
In-Reply-To: <57769188.9060708@sr71.net> (Dave Hansen's message of "Fri, 1 Jul 2016 08:51:36 -0700")

Dave Hansen <dave@sr71.net> writes:

> On 07/01/2016 07:25 AM, Eric W. Biederman wrote:
>> Linus Torvalds <torvalds@linux-foundation.org> writes:
>>> > On Thu, Jun 30, 2016 at 9:39 PM, Dave Hansen <dave@sr71.net> wrote:
>>>> >>
>>>> >> I think what you suggest will work if we don't consider A/D in
>>>> >> pte_none().  I think there are a bunch of code path where assume that
>>>> >> !pte_present() && !pte_none() means swap.
>>> >
>>> > Yeah, we would need to change pte_none() to mask off D/A, but I think
>>> > that might be the only real change needed (other than making sure that
>>> > we don't use the bits in the swap entries, I didn't look at that part
>>> > at all)
>> It looks like __pte_to_swp_entry also needs to be changed to mask out
>> those bits when the swap code reads pte entries.  For all of the same
>> reasons as pte_none.
>
> I guess that would be nice, but isn't it redundant?
>
> static inline swp_entry_t pte_to_swp_entry(pte_t pte)
> {
> 	...
>         arch_entry = __pte_to_swp_entry(pte);
> 	return swp_entry(__swp_type(arch_entry), __swp_offset(arch_entry));
> }
>
> As long as __swp_type() and __swp_offset() don't let A/D through, then
> we should be OK.  This site is the only call to __pte_to_swp_entry()
> that I can find in the entire codebase.
>
> Or am I missing something?

Given that __pte_to_swp_entry on x86_64 is just __pte_val or pte.pte it
does no filtering.  Similarly __swp_type(arch_entry) is a >> and
swp_entry(type, ...) is a << of what appears to be same amount
for the swap type.

So any corruption in the upper bits of the pte will be preserved as a
swap type.

In fact I strongly suspect that the compiler can optimize out all of the
work done by "swp_entry(__swp_type(arch_entry), _swp_offset(arch_entry))".

Eric

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: ebiederm@xmission.com (Eric W. Biederman)
To: Dave Hansen <dave@sr71.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	the arch/x86 maintainers <x86@kernel.org>,
	linux-mm <linux-mm@kvack.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Borislav Petkov <bp@alien8.de>, Andi Kleen <ak@linux.intel.com>,
	Michal Hocko <mhocko@suse.com>,
	Dave Hansen <dave.hansen@linux.intel.com>
Subject: Re: [PATCH 6/6] x86: Fix stray A/D bit setting into non-present PTEs
Date: Fri, 01 Jul 2016 13:12:55 -0500	[thread overview]
Message-ID: <878txlb520.fsf@x220.int.ebiederm.org> (raw)
In-Reply-To: <57769188.9060708@sr71.net> (Dave Hansen's message of "Fri, 1 Jul 2016 08:51:36 -0700")

Dave Hansen <dave@sr71.net> writes:

> On 07/01/2016 07:25 AM, Eric W. Biederman wrote:
>> Linus Torvalds <torvalds@linux-foundation.org> writes:
>>> > On Thu, Jun 30, 2016 at 9:39 PM, Dave Hansen <dave@sr71.net> wrote:
>>>> >>
>>>> >> I think what you suggest will work if we don't consider A/D in
>>>> >> pte_none().  I think there are a bunch of code path where assume that
>>>> >> !pte_present() && !pte_none() means swap.
>>> >
>>> > Yeah, we would need to change pte_none() to mask off D/A, but I think
>>> > that might be the only real change needed (other than making sure that
>>> > we don't use the bits in the swap entries, I didn't look at that part
>>> > at all)
>> It looks like __pte_to_swp_entry also needs to be changed to mask out
>> those bits when the swap code reads pte entries.  For all of the same
>> reasons as pte_none.
>
> I guess that would be nice, but isn't it redundant?
>
> static inline swp_entry_t pte_to_swp_entry(pte_t pte)
> {
> 	...
>         arch_entry = __pte_to_swp_entry(pte);
> 	return swp_entry(__swp_type(arch_entry), __swp_offset(arch_entry));
> }
>
> As long as __swp_type() and __swp_offset() don't let A/D through, then
> we should be OK.  This site is the only call to __pte_to_swp_entry()
> that I can find in the entire codebase.
>
> Or am I missing something?

Given that __pte_to_swp_entry on x86_64 is just __pte_val or pte.pte it
does no filtering.  Similarly __swp_type(arch_entry) is a >> and
swp_entry(type, ...) is a << of what appears to be same amount
for the swap type.

So any corruption in the upper bits of the pte will be preserved as a
swap type.

In fact I strongly suspect that the compiler can optimize out all of the
work done by "swp_entry(__swp_type(arch_entry), _swp_offset(arch_entry))".

Eric

  reply	other threads:[~2016-07-01 18:25 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-01  0:12 [PATCH 0/6] [v3] Workaround for Xeon Phi PTE A/D bits erratum Dave Hansen
2016-07-01  0:12 ` Dave Hansen
2016-07-01  0:12 ` [PATCH 1/6] x86: fix duplicated X86_BUG(9) macro Dave Hansen
2016-07-01  0:12   ` Dave Hansen
2016-07-01  0:12   ` Dave Hansen
2016-07-01  9:23   ` Borislav Petkov
2016-07-01 16:30     ` Andy Lutomirski
2016-07-01 16:30       ` Andy Lutomirski
2016-07-01 16:46       ` Borislav Petkov
2016-07-03 14:36         ` Andy Lutomirski
2016-07-03 14:36           ` Andy Lutomirski
2016-07-03 18:44           ` Borislav Petkov
2016-07-01  0:12 ` [PATCH 2/6] mm, tlb: add mmu_gather->saw_unset_a_or_d Dave Hansen
2016-07-01  0:12   ` Dave Hansen
2016-07-01  0:12 ` [PATCH 3/6] mm: add force_batch_flush to mmu_gather Dave Hansen
2016-07-01  0:12   ` Dave Hansen
2016-07-01  0:12 ` [PATCH 4/6] mm: move flush in madvise_free_pte_range() Dave Hansen
2016-07-01  0:12   ` Dave Hansen
2016-07-01  0:12 ` [PATCH 5/6] mm: make tlb_flush_mmu_tlbonly() return whether it flushed Dave Hansen
2016-07-01  0:12   ` Dave Hansen
2016-07-01  0:12 ` [PATCH 6/6] x86: Fix stray A/D bit setting into non-present PTEs Dave Hansen
2016-07-01  0:12   ` Dave Hansen
2016-07-01  1:50   ` Nadav Amit
2016-07-01  1:50     ` Nadav Amit
2016-07-01  1:54     ` Dave Hansen
2016-07-01  1:54       ` Dave Hansen
2016-07-01  2:55   ` Linus Torvalds
2016-07-01  2:55     ` Linus Torvalds
2016-07-01  3:06     ` Brian Gerst
2016-07-01  3:06       ` Brian Gerst
2016-07-01  3:21       ` Linus Torvalds
2016-07-03 17:10       ` Dave Hansen
2016-07-03 17:10         ` Dave Hansen
2016-07-01  4:39     ` Dave Hansen
2016-07-01  4:39       ` Dave Hansen
2016-07-01  5:43       ` Linus Torvalds
2016-07-01  5:43         ` Linus Torvalds
2016-07-01 14:25         ` Eric W. Biederman
2016-07-01 14:25           ` Eric W. Biederman
2016-07-01 15:51           ` Dave Hansen
2016-07-01 15:51             ` Dave Hansen
2016-07-01 18:12             ` Eric W. Biederman [this message]
2016-07-01 18:12               ` Eric W. Biederman
2016-07-01 16:07       ` Linus Torvalds
2016-07-01 16:07         ` Linus Torvalds
2016-07-01 16:14         ` Dave Hansen
2016-07-01 16:14           ` Dave Hansen
2016-07-01 16:25           ` Linus Torvalds
2016-07-01 16:25             ` Linus Torvalds

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=878txlb520.fsf@x220.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=ak@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=dave@sr71.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --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.