Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1305067 > unrolled thread

[RFC 01/13] x86/paravirt: Turn KASAN off for parvirt.o

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC 01/13] x86/paravirt: Turn KASAN off for parvirt.o Andy Lutomirski <luto@kernel.org> - 2016-01-09 00:20 +0100
    Re: [RFC 01/13] x86/paravirt: Turn KASAN off for parvirt.o Borislav Petkov <bp@alien8.de> - 2016-01-10 20:10 +0100
      [PATCH 2/2] x86/kasan: write protect kasan zero shadow Andrey Ryabinin <aryabinin@virtuozzo.com> - 2016-01-11 14:00 +0100
      Re: [RFC 01/13] x86/paravirt: Turn KASAN off for parvirt.o Andrey Ryabinin <aryabinin@virtuozzo.com> - 2016-01-11 14:00 +0100
        [PATCH 1/2] x86/kasan: clear kasan_zero_page after TLB flush Andrey Ryabinin <aryabinin@virtuozzo.com> - 2016-01-11 14:00 +0100

#1305067 — [RFC 01/13] x86/paravirt: Turn KASAN off for parvirt.o

Otherwise terrible things happen if some of the callbacks end up
calling into KASAN in unexpected places.

This has no obvious symptoms yet, but adding a memory reference to
native_flush_tlb_global without this blows up on KASAN kernels.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index b1b78ffe01d0..b7cd5bdf314b 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -19,6 +19,7 @@ endif
 KASAN_SANITIZE_head$(BITS).o := n
 KASAN_SANITIZE_dumpstack.o := n
 KASAN_SANITIZE_dumpstack_$(BITS).o := n
+KASAN_SANITIZE_paravirt.o := n
 
 CFLAGS_irq.o := -I$(src)/../include/asm/trace
 
-- 
2.5.0

[toc] | [next] | [standalone]

#1305655

+ Andrey.

On Fri, Jan 08, 2016 at 03:15:19PM -0800, Andy Lutomirski wrote:
> Otherwise terrible things happen if some of the callbacks end up
> calling into KASAN in unexpected places.
> 
> This has no obvious symptoms yet, but adding a memory reference to
> native_flush_tlb_global without this blows up on KASAN kernels.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/kernel/Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index b1b78ffe01d0..b7cd5bdf314b 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -19,6 +19,7 @@ endif
>  KASAN_SANITIZE_head$(BITS).o := n
>  KASAN_SANITIZE_dumpstack.o := n
>  KASAN_SANITIZE_dumpstack_$(BITS).o := n
> +KASAN_SANITIZE_paravirt.o := n
>  
>  CFLAGS_irq.o := -I$(src)/../include/asm/trace

Shouldn't we take this one irrespectively of what happens to the rest in
the patchset?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1306157 — [PATCH 2/2] x86/kasan: write protect kasan zero shadow

After kasan_init() executed, no one is allowed to write to kasan_zero_page,
so write protect it.

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 arch/x86/mm/kasan_init_64.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/kasan_init_64.c b/arch/x86/mm/kasan_init_64.c
index 303e470..1b1110f 100644
--- a/arch/x86/mm/kasan_init_64.c
+++ b/arch/x86/mm/kasan_init_64.c
@@ -125,10 +125,16 @@ void __init kasan_init(void)
 
 	/*
 	 * kasan_zero_page has been used as early shadow memory, thus it may
-	 * contain some garbage. Now we can clear it, since after the TLB flush
-	 * no one should write to it.
+	 * contain some garbage. Now we can clear and write protect it, since
+	 * after the TLB flush no one should write to it.
 	 */
 	memset(kasan_zero_page, 0, PAGE_SIZE);
+	for (i = 0; i < PTRS_PER_PTE; i++) {
+		pte_t pte = __pte(__pa(kasan_zero_page) | __PAGE_KERNEL_RO);
+		set_pte(&kasan_zero_pte[i], pte);
+	}
+	/* Flush TLBs again to be sure that write protection applied. */
+	__flush_tlb_all();
 
 	init_task.kasan_depth = 0;
 	pr_info("KernelAddressSanitizer initialized\n");
-- 
2.4.10

[toc] | [prev] | [next] | [standalone]

#1306160

On 01/10/2016 09:59 PM, Borislav Petkov wrote:
> + Andrey.
> 
> On Fri, Jan 08, 2016 at 03:15:19PM -0800, Andy Lutomirski wrote:
>> Otherwise terrible things happen if some of the callbacks end up
>> calling into KASAN in unexpected places.
>>
>> This has no obvious symptoms yet, but adding a memory reference to
>> native_flush_tlb_global without this blows up on KASAN kernels.
>>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>  arch/x86/kernel/Makefile | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
>> index b1b78ffe01d0..b7cd5bdf314b 100644
>> --- a/arch/x86/kernel/Makefile
>> +++ b/arch/x86/kernel/Makefile
>> @@ -19,6 +19,7 @@ endif
>>  KASAN_SANITIZE_head$(BITS).o := n
>>  KASAN_SANITIZE_dumpstack.o := n
>>  KASAN_SANITIZE_dumpstack_$(BITS).o := n
>> +KASAN_SANITIZE_paravirt.o := n
>>  
>>  CFLAGS_irq.o := -I$(src)/../include/asm/trace
> 
> Shouldn't we take this one irrespectively of what happens to the rest in
> the patchset?
>

I don't think that this patch is the right way to solve the problem.
The follow-up patch "x86/kasan: clear kasan_zero_page after TLB flush" should fix Andy's problem.

[toc] | [prev] | [next] | [standalone]

#1306164 — [PATCH 1/2] x86/kasan: clear kasan_zero_page after TLB flush

Currently we clear kasan_zero_page before __flush_tlb_all(). This
works with current implementation of native_flush_tlb[_global]()
because it doesn't cause do any writes to kasan shadow memory.
But any subtle change made in native_flush_tlb*() could break this.
Also current code seems doesn't work for paravirt guests (lguest).

Only after the TLB flush we can be sure that kasan_zero_page is not
used as early shadow anymore (instrumented code will not write to it).
So it should cleared it only after the TLB flush.

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 arch/x86/mm/kasan_init_64.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/x86/mm/kasan_init_64.c b/arch/x86/mm/kasan_init_64.c
index d470cf2..303e470 100644
--- a/arch/x86/mm/kasan_init_64.c
+++ b/arch/x86/mm/kasan_init_64.c
@@ -120,11 +120,16 @@ void __init kasan_init(void)
 	kasan_populate_zero_shadow(kasan_mem_to_shadow((void *)MODULES_END),
 			(void *)KASAN_SHADOW_END);
 
-	memset(kasan_zero_page, 0, PAGE_SIZE);
-
 	load_cr3(init_level4_pgt);
 	__flush_tlb_all();
-	init_task.kasan_depth = 0;
 
+	/*
+	 * kasan_zero_page has been used as early shadow memory, thus it may
+	 * contain some garbage. Now we can clear it, since after the TLB flush
+	 * no one should write to it.
+	 */
+	memset(kasan_zero_page, 0, PAGE_SIZE);
+
+	init_task.kasan_depth = 0;
 	pr_info("KernelAddressSanitizer initialized\n");
 }
-- 
2.4.10

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web