From: Andrey Ryabinin <aryabinin@virtuozzo.com>
To: Daniel Axtens <dja@axtens.net>,
kasan-dev@googlegroups.com, linux-mm@kvack.org, x86@kernel.org,
glider@google.com, luto@kernel.org, linux-kernel@vger.kernel.org,
mark.rutland@arm.com, dvyukov@google.com,
christophe.leroy@c-s.fr
Cc: linuxppc-dev@lists.ozlabs.org, gor@linux.ibm.com
Subject: Re: [PATCH v8 1/5] kasan: support backing vmalloc space with real shadow memory
Date: Wed, 16 Oct 2019 15:19:50 +0300 [thread overview]
Message-ID: <8f573b40-3a5a-ed36-dffb-4a54faf3c4e1@virtuozzo.com> (raw)
In-Reply-To: <87ftjvtoo7.fsf@dja-thinkpad.axtens.net>
On 10/14/19 4:57 PM, Daniel Axtens wrote:
> Hi Andrey,
>
>
>>> + /*
>>> + * Ensure poisoning is visible before the shadow is made visible
>>> + * to other CPUs.
>>> + */
>>> + smp_wmb();
>>
>> I'm not quite understand what this barrier do and why it needed.
>> And if it's really needed there should be a pairing barrier
>> on the other side which I don't see.
>
> Mark might be better able to answer this, but my understanding is that
> we want to make sure that we never have a situation where the writes are
> reordered so that PTE is installed before all the poisioning is written
> out. I think it follows the logic in __pte_alloc() in mm/memory.c:
>
> /*
> * Ensure all pte setup (eg. pte page lock and page clearing) are
> * visible before the pte is made visible to other CPUs by being
> * put into page tables.
> *
> * The other side of the story is the pointer chasing in the page
> * table walking code (when walking the page table without locking;
> * ie. most of the time). Fortunately, these data accesses consist
> * of a chain of data-dependent loads, meaning most CPUs (alpha
> * being the notable exception) will already guarantee loads are
> * seen in-order. See the alpha page table accessors for the
> * smp_read_barrier_depends() barriers in page table walking code.
> */
> smp_wmb(); /* Could be smp_wmb__xxx(before|after)_spin_lock */
>
> I can clarify the comment.
>
I don't see how is this relevant here.
barrier in __pte_alloc() for very the following case:
CPU 0 CPU 1
__pte_alloc(): pte_offset_kernel(pmd_t * dir, unsigned long address):
pgtable_t new = pte_alloc_one(mm); pte_t *new = (pte_t *) pmd_page_vaddr(*dir) + ((address >> PAGE_SHIFT) & (PTRS_PER_PAGE - 1));
smp_wmb(); smp_read_barrier_depends();
pmd_populate(mm, pmd, new);
/* do something with pte, e.g. check if (pte_none(*new)) */
It's needed to ensure that if CPU1 sees pmd_populate() it also sees initialized contents of the 'new'.
In our case the barrier would have been needed if we had the other side like this:
if (!pte_none(*vmalloc_shadow_pte)) {
shadow_addr = (unsigned long)__va(pte_pfn(*vmalloc_shadow_pte) << PAGE_SHIFT);
smp_read_barrier_depends();
*shadow_addr; /* read the shadow, barrier ensures that if we see installed pte, we will see initialized shadow memory. */
}
Without such other side the barrier is pointless.
next prev parent reply other threads:[~2019-10-16 12:20 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-01 6:58 [PATCH v8 0/5] kasan: support backing vmalloc space with real shadow memory Daniel Axtens
2019-10-01 6:58 ` [PATCH v8 1/5] " Daniel Axtens
2019-10-01 10:17 ` Uladzislau Rezki
2019-10-02 1:23 ` Daniel Axtens
2019-10-02 7:13 ` Christophe Leroy
2019-10-02 11:49 ` Uladzislau Rezki
2019-10-07 8:02 ` Uladzislau Rezki
2019-10-11 5:15 ` Daniel Axtens
2019-10-11 19:57 ` Andrey Ryabinin
2019-10-14 13:57 ` Daniel Axtens
2019-10-14 15:27 ` Mark Rutland
2019-10-15 6:32 ` Daniel Axtens
2019-10-15 6:29 ` Daniel Axtens
2019-10-16 12:19 ` Andrey Ryabinin [this message]
2019-10-16 13:22 ` Mark Rutland
2019-10-18 10:43 ` Andrey Ryabinin
2019-10-28 7:39 ` Daniel Axtens
2019-10-28 1:26 ` Daniel Axtens
2019-10-14 15:43 ` Mark Rutland
2019-10-15 6:27 ` Daniel Axtens
2019-10-01 6:58 ` [PATCH v8 2/5] kasan: add test for vmalloc Daniel Axtens
2019-10-01 6:58 ` [PATCH v8 3/5] fork: support VMAP_STACK with KASAN_VMALLOC Daniel Axtens
2019-10-01 6:58 ` [PATCH v8 4/5] x86/kasan: support KASAN_VMALLOC Daniel Axtens
2019-10-01 6:58 ` [PATCH v8 5/5] kasan debug: track pages allocated for vmalloc shadow Daniel Axtens
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=8f573b40-3a5a-ed36-dffb-4a54faf3c4e1@virtuozzo.com \
--to=aryabinin@virtuozzo.com \
--cc=christophe.leroy@c-s.fr \
--cc=dja@axtens.net \
--cc=dvyukov@google.com \
--cc=glider@google.com \
--cc=gor@linux.ibm.com \
--cc=kasan-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=luto@kernel.org \
--cc=mark.rutland@arm.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).