All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Axtens <dja@axtens.net>
To: Andrey Ryabinin <aryabinin@virtuozzo.com>,
	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,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v10 4/5] x86/kasan: support KASAN_VMALLOC
Date: Thu, 31 Oct 2019 00:50:05 +1100	[thread overview]
Message-ID: <87sgnamjg2.fsf@dja-thinkpad.axtens.net> (raw)
In-Reply-To: <a144eaca-d7e1-1a18-5975-bd0bfdb9450e@virtuozzo.com>

Andrey Ryabinin <aryabinin@virtuozzo.com> writes:

> On 10/29/19 7:20 AM, Daniel Axtens wrote:
>> In the case where KASAN directly allocates memory to back vmalloc
>> space, don't map the early shadow page over it.
>> 
>> We prepopulate pgds/p4ds for the range that would otherwise be empty.
>> This is required to get it synced to hardware on boot, allowing the
>> lower levels of the page tables to be filled dynamically.
>> 
>> Acked-by: Dmitry Vyukov <dvyukov@google.com>
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>> 
>> ---
>
>> +static void __init kasan_shallow_populate_pgds(void *start, void *end)
>> +{
>> +	unsigned long addr, next;
>> +	pgd_t *pgd;
>> +	void *p;
>> +	int nid = early_pfn_to_nid((unsigned long)start);
>
> This doesn't make sense. start is not even a pfn. With linear mapping 
> we try to identify nid to have the shadow on the same node as memory. But 
> in this case we don't have memory or the corresponding shadow (yet),
> we only install pgd/p4d.
> I guess we could just use NUMA_NO_NODE.

Ah wow, that's quite the clanger on my part.

There are a couple of other invocations of early_pfn_to_nid in that file
that use an address directly, but at least they reference actual memory.
I'll send a separate patch to fix those up.

> The rest looks ok, so with that fixed:
>
> Reviewed-by: Andrey Ryabinin <aryabinin@virtuozzo.com>

Thanks heaps! I've fixed up the nit you identifed in the first patch,
and I agree that the last patch probably isn't needed. I'll respin the
series shortly.

Regards,
Daniel

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Axtens <dja@axtens.net>
To: Andrey Ryabinin <aryabinin@virtuozzo.com>,
	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: Andrew Morton <akpm@linux-foundation.org>,
	linuxppc-dev@lists.ozlabs.org, gor@linux.ibm.com
Subject: Re: [PATCH v10 4/5] x86/kasan: support KASAN_VMALLOC
Date: Thu, 31 Oct 2019 00:50:05 +1100	[thread overview]
Message-ID: <87sgnamjg2.fsf@dja-thinkpad.axtens.net> (raw)
In-Reply-To: <a144eaca-d7e1-1a18-5975-bd0bfdb9450e@virtuozzo.com>

Andrey Ryabinin <aryabinin@virtuozzo.com> writes:

> On 10/29/19 7:20 AM, Daniel Axtens wrote:
>> In the case where KASAN directly allocates memory to back vmalloc
>> space, don't map the early shadow page over it.
>> 
>> We prepopulate pgds/p4ds for the range that would otherwise be empty.
>> This is required to get it synced to hardware on boot, allowing the
>> lower levels of the page tables to be filled dynamically.
>> 
>> Acked-by: Dmitry Vyukov <dvyukov@google.com>
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>> 
>> ---
>
>> +static void __init kasan_shallow_populate_pgds(void *start, void *end)
>> +{
>> +	unsigned long addr, next;
>> +	pgd_t *pgd;
>> +	void *p;
>> +	int nid = early_pfn_to_nid((unsigned long)start);
>
> This doesn't make sense. start is not even a pfn. With linear mapping 
> we try to identify nid to have the shadow on the same node as memory. But 
> in this case we don't have memory or the corresponding shadow (yet),
> we only install pgd/p4d.
> I guess we could just use NUMA_NO_NODE.

Ah wow, that's quite the clanger on my part.

There are a couple of other invocations of early_pfn_to_nid in that file
that use an address directly, but at least they reference actual memory.
I'll send a separate patch to fix those up.

> The rest looks ok, so with that fixed:
>
> Reviewed-by: Andrey Ryabinin <aryabinin@virtuozzo.com>

Thanks heaps! I've fixed up the nit you identifed in the first patch,
and I agree that the last patch probably isn't needed. I'll respin the
series shortly.

Regards,
Daniel

  reply	other threads:[~2019-10-30 13:50 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-29  4:20 [PATCH v10 0/5] kasan: support backing vmalloc space with real shadow memory Daniel Axtens
2019-10-29  4:20 ` [PATCH v10 1/5] " Daniel Axtens
2019-10-29 16:42   ` Andrey Ryabinin
2019-10-29 16:42     ` Andrey Ryabinin
2019-10-30 14:29   ` Uladzislau Rezki
2019-10-30 14:29     ` Uladzislau Rezki
2019-10-31  9:36     ` Daniel Axtens
2019-10-31  9:36       ` Daniel Axtens
2019-10-29  4:20 ` [PATCH v10 2/5] kasan: add test for vmalloc Daniel Axtens
2019-10-29 16:43   ` Andrey Ryabinin
2019-10-29  4:20 ` [PATCH v10 3/5] fork: support VMAP_STACK with KASAN_VMALLOC Daniel Axtens
2019-10-29 17:07   ` Andrey Ryabinin
2019-10-29 17:07     ` Andrey Ryabinin
2019-10-29  4:20 ` [PATCH v10 4/5] x86/kasan: support KASAN_VMALLOC Daniel Axtens
2019-10-29 17:21   ` Andrey Ryabinin
2019-10-29 17:21     ` Andrey Ryabinin
2019-10-30 13:50     ` Daniel Axtens [this message]
2019-10-30 13:50       ` Daniel Axtens
2019-10-30 14:12       ` Andrey Ryabinin
2019-10-30 14:12         ` Andrey Ryabinin
2019-10-30 14:21         ` Daniel Axtens
2019-10-30 14:21           ` Daniel Axtens
2019-10-29  4:20 ` [PATCH v10 5/5] kasan debug: track pages allocated for vmalloc shadow Daniel Axtens
2019-10-29 17:34   ` Andrey Ryabinin
2019-10-29 17:34     ` Andrey Ryabinin

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=87sgnamjg2.fsf@dja-thinkpad.axtens.net \
    --to=dja@axtens.net \
    --cc=akpm@linux-foundation.org \
    --cc=aryabinin@virtuozzo.com \
    --cc=christophe.leroy@c-s.fr \
    --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 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.