Re: [PATCH v6 0/6] KASAN for arm64

[Andrey Ryabinin]

2015-10-09 15:42 GMT+03:00 Mark Rutland :
> On Fri, Oct 09, 2015 at 01:18:09PM +0300, Andrey Ryabinin wrote:
>> 2015-10-09 12:48 GMT+03:00 Mark Rutland :
>> > On Fri, Oct 09, 2015 at 12:32:18PM +0300, Andrey Ryabinin wrote:
>> > [...]
>> >
>> >> I thought the EFI stub isolation patches create a copy of mem*() 
>> >> functions in the stub,
>> >> but they are just create aliases with __efistub_ prefix.
>> >>
>> >> We only need to create some more aliases for KASAN.
>> >> The following patch on top of the EFI stub isolation series works for me.
>> >>
>> >>
>> >> Signed-off-by: Andrey Ryabinin 
>> >> ---
>> >>  arch/arm64/kernel/image.h | 6 ++
>> >>  1 file changed, 6 insertions(+)
>> >>
>> >> diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
>> >> index e083af0..6eb8fee 100644
>> >> --- a/arch/arm64/kernel/image.h
>> >> +++ b/arch/arm64/kernel/image.h
>> >> @@ -80,6 +80,12 @@ __efistub_strcmp   = __pi_strcmp;
>> >>  __efistub_strncmp= __pi_strncmp;
>> >>  __efistub___flush_dcache_area= __pi___flush_dcache_area;
>> >>
>> >> +#ifdef CONFIG_KASAN
>> >> +__efistub___memcpy   = __pi_memcpy;
>> >> +__efistub___memmove  = __pi_memmove;
>> >> +__efistub___memset   = __pi_memset;
>> >> +#endif
>> >
>> > Ard's v4 stub isolation series has these aliases [1], as the stub
>> > requires these aliases regardless of KASAN in order to link.
>>
>> Stub isolation series has __efistub_memcpy, not __efistub___memcpy
>> (two additional '_').
>
> Ah, I see, sorry for my sloppy reading.
>
>> The thing is, KASAN provides own implementation of memcpy() which
>> checks memory before access.
>> The original 'memcpy()' becomes __memcpy(), so we could still use it.
>
> Ok.
>
>> In code that not instrumented by KASAN (like the EFI stub) we replace
>> KASAN's memcpy() with the original __mempcy():
>> #define memcpy() __memcpy()
>
> I'm a little confused by this. Surely that doesn't override implicit
> calls generated by the compiler, leaving us with a mixture of calls to
> memcpy and __memcpy?
>
> That doesn't matter for the stub, as both __efistub_mem* and
> __efistub___mem* would point at __pe_mem*, but doesn't that matter for
> other users that shouldn't be instrumented?
>
> Is that not a problem, or do we inhibit/override that somehow?
>

You are right, GCC could emit memcpy() call. It's just not a problem so far.
The amount of not instrumented code is fairly small (some low-level
x86 code, kasan internals and slub allocator).

The purpose of these defines is to not spread kasan-specific details
across unrelated code.
E.g. there are a lot of memcpy()/memset() calls in slub that used to
access object's redzone or
freed objects. So it simpler to redefine memset, rather then somehow
mangle that code.

>> So with CONFIG_KASAN=y the EFI stub uses __memcpy, thus we need to
>> create the __efistub___memcpy alias.
>
> Ok, that makes sense to me.
>
> Thanks,
> Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Mark Rutland]

On Fri, Oct 09, 2015 at 01:18:09PM +0300, Andrey Ryabinin wrote:
> 2015-10-09 12:48 GMT+03:00 Mark Rutland :
> > On Fri, Oct 09, 2015 at 12:32:18PM +0300, Andrey Ryabinin wrote:
> > [...]
> >
> >> I thought the EFI stub isolation patches create a copy of mem*() functions 
> >> in the stub,
> >> but they are just create aliases with __efistub_ prefix.
> >>
> >> We only need to create some more aliases for KASAN.
> >> The following patch on top of the EFI stub isolation series works for me.
> >>
> >>
> >> Signed-off-by: Andrey Ryabinin 
> >> ---
> >>  arch/arm64/kernel/image.h | 6 ++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
> >> index e083af0..6eb8fee 100644
> >> --- a/arch/arm64/kernel/image.h
> >> +++ b/arch/arm64/kernel/image.h
> >> @@ -80,6 +80,12 @@ __efistub_strcmp   = __pi_strcmp;
> >>  __efistub_strncmp= __pi_strncmp;
> >>  __efistub___flush_dcache_area= __pi___flush_dcache_area;
> >>
> >> +#ifdef CONFIG_KASAN
> >> +__efistub___memcpy   = __pi_memcpy;
> >> +__efistub___memmove  = __pi_memmove;
> >> +__efistub___memset   = __pi_memset;
> >> +#endif
> >
> > Ard's v4 stub isolation series has these aliases [1], as the stub
> > requires these aliases regardless of KASAN in order to link.
> 
> Stub isolation series has __efistub_memcpy, not __efistub___memcpy
> (two additional '_').

Ah, I see, sorry for my sloppy reading.

> The thing is, KASAN provides own implementation of memcpy() which
> checks memory before access.
> The original 'memcpy()' becomes __memcpy(), so we could still use it.

Ok.

> In code that not instrumented by KASAN (like the EFI stub) we replace
> KASAN's memcpy() with the original __mempcy():
> #define memcpy() __memcpy()

I'm a little confused by this. Surely that doesn't override implicit
calls generated by the compiler, leaving us with a mixture of calls to
memcpy and __memcpy?

That doesn't matter for the stub, as both __efistub_mem* and
__efistub___mem* would point at __pe_mem*, but doesn't that matter for
other users that shouldn't be instrumented?

Is that not a problem, or do we inhibit/override that somehow?

> So with CONFIG_KASAN=y the EFI stub uses __memcpy, thus we need to
> create the __efistub___memcpy alias.

Ok, that makes sense to me.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Andrey Ryabinin]

2015-10-09 12:48 GMT+03:00 Mark Rutland :
> On Fri, Oct 09, 2015 at 12:32:18PM +0300, Andrey Ryabinin wrote:
> [...]
>
>> I thought the EFI stub isolation patches create a copy of mem*() functions 
>> in the stub,
>> but they are just create aliases with __efistub_ prefix.
>>
>> We only need to create some more aliases for KASAN.
>> The following patch on top of the EFI stub isolation series works for me.
>>
>>
>> Signed-off-by: Andrey Ryabinin 
>> ---
>>  arch/arm64/kernel/image.h | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
>> index e083af0..6eb8fee 100644
>> --- a/arch/arm64/kernel/image.h
>> +++ b/arch/arm64/kernel/image.h
>> @@ -80,6 +80,12 @@ __efistub_strcmp   = __pi_strcmp;
>>  __efistub_strncmp= __pi_strncmp;
>>  __efistub___flush_dcache_area= __pi___flush_dcache_area;
>>
>> +#ifdef CONFIG_KASAN
>> +__efistub___memcpy   = __pi_memcpy;
>> +__efistub___memmove  = __pi_memmove;
>> +__efistub___memset   = __pi_memset;
>> +#endif
>
> Ard's v4 stub isolation series has these aliases [1], as the stub
> requires these aliases regardless of KASAN in order to link.

Stub isolation series has __efistub_memcpy, not __efistub___memcpy
(two additional '_').
The thing is, KASAN provides own implementation of memcpy() which
checks memory before access.
The original 'memcpy()' becomes __memcpy(), so we could still use it.
In code that not instrumented by KASAN (like the EFI stub) we replace
KASAN's memcpy() with the original __mempcy():
#define memcpy() __memcpy()

So with CONFIG_KASAN=y the EFI stub uses __memcpy, thus we need to
create the __efistub___memcpy alias.

>
> Thanks,
> Mark.
>
> [1] 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/375708.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Mark Rutland]

On Fri, Oct 09, 2015 at 12:32:18PM +0300, Andrey Ryabinin wrote:
[...]

> I thought the EFI stub isolation patches create a copy of mem*() functions in 
> the stub,
> but they are just create aliases with __efistub_ prefix.
> 
> We only need to create some more aliases for KASAN.
> The following patch on top of the EFI stub isolation series works for me.
> 
> 
> Signed-off-by: Andrey Ryabinin 
> ---
>  arch/arm64/kernel/image.h | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
> index e083af0..6eb8fee 100644
> --- a/arch/arm64/kernel/image.h
> +++ b/arch/arm64/kernel/image.h
> @@ -80,6 +80,12 @@ __efistub_strcmp   = __pi_strcmp;
>  __efistub_strncmp= __pi_strncmp;
>  __efistub___flush_dcache_area= __pi___flush_dcache_area;
>  
> +#ifdef CONFIG_KASAN
> +__efistub___memcpy   = __pi_memcpy;
> +__efistub___memmove  = __pi_memmove;
> +__efistub___memset   = __pi_memset;
> +#endif

Ard's v4 stub isolation series has these aliases [1], as the stub
requires these aliases regardless of KASAN in order to link.

Thanks,
Mark.

[1] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/375708.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Andrey Ryabinin]

On 10/08/2015 07:07 PM, Andrey Ryabinin wrote:
> 2015-10-08 18:11 GMT+03:00 Catalin Marinas :
>> On Thu, Oct 08, 2015 at 02:09:26PM +0200, Ard Biesheuvel wrote:
>>> On 8 October 2015 at 13:23, Andrey Ryabinin  wrote:
 On 10/08/2015 02:11 PM, Mark Rutland wrote:
> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
>>> On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
 As usual patches available in git
   git://github.com/aryabinin/linux.git kasan/arm64v6

 Changes since v5:
  - Rebase on top of 4.3-rc1
  - Fixed EFI boot.
  - Updated Doc/features/KASAN.
>>>
>>> I tried to merge these patches (apart from the x86 one which is already
>>> merged) but it still doesn't boot on Juno as an EFI application.
>>>
>>
>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
>> regions")
>> It introduced sort() call in efi_get_virtmap().
>> sort() is generic kernel function and it's instrumented, so we crash
>> when KASAN tries to access shadow in sort().
>
> I believe this is solved by Ard's stub isolation series [1,2], which
> will build a stub-specific copy of sort() and various other functions
> (see the arm-deps in [2]).
>
> So long as the stub is not built with ASAN, that should work.

 Thanks, this should help, as we already build the stub without ASAN 
 instrumentation.
>>>
>>> Indeed. I did not mention instrumentation in the commit log for those
>>> patches, but obviously, something like KASAN instrumentation cannot be
>>> tolerated in the stub since it makes assumptions about the memory
>>> layout
>>
>> I'll review your latest EFI stub isolation patches and try Kasan again
>> on top (most likely tomorrow).
> 
> You'd better wait for v7, because kasan patches will need some adjustment.
> Since stub is isolated,  we need to handle memcpy vs __memcpy stuff the same
> way as we do in x86. Now we also need to #undef memset/memcpy/memmove in ARM64
> (just like this was done for x86).
> 

Hm, I was wrong, we don't need that.

I thought the EFI stub isolation patches create a copy of mem*() functions in 
the stub,
but they are just create aliases with __efistub_ prefix.

We only need to create some more aliases for KASAN.
The following patch on top of the EFI stub isolation series works for me.


Signed-off-by: Andrey Ryabinin 
---
 arch/arm64/kernel/image.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
index e083af0..6eb8fee 100644
--- a/arch/arm64/kernel/image.h
+++ b/arch/arm64/kernel/image.h
@@ -80,6 +80,12 @@ __efistub_strcmp = __pi_strcmp;
 __efistub_strncmp  = __pi_strncmp;
 __efistub___flush_dcache_area  = __pi___flush_dcache_area;
 
+#ifdef CONFIG_KASAN
+__efistub___memcpy = __pi_memcpy;
+__efistub___memmove= __pi_memmove;
+__efistub___memset = __pi_memset;
+#endif
+
 __efistub__text= _text;
 __efistub__end = _end;
 __efistub__edata   = _edata;
-- 
2.4.9






--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Andrey Ryabinin]

On 10/08/2015 07:07 PM, Andrey Ryabinin wrote:
> 2015-10-08 18:11 GMT+03:00 Catalin Marinas :
>> On Thu, Oct 08, 2015 at 02:09:26PM +0200, Ard Biesheuvel wrote:
>>> On 8 October 2015 at 13:23, Andrey Ryabinin  wrote:
 On 10/08/2015 02:11 PM, Mark Rutland wrote:
> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
>>> On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
 As usual patches available in git
   git://github.com/aryabinin/linux.git kasan/arm64v6

 Changes since v5:
  - Rebase on top of 4.3-rc1
  - Fixed EFI boot.
  - Updated Doc/features/KASAN.
>>>
>>> I tried to merge these patches (apart from the x86 one which is already
>>> merged) but it still doesn't boot on Juno as an EFI application.
>>>
>>
>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
>> regions")
>> It introduced sort() call in efi_get_virtmap().
>> sort() is generic kernel function and it's instrumented, so we crash
>> when KASAN tries to access shadow in sort().
>
> I believe this is solved by Ard's stub isolation series [1,2], which
> will build a stub-specific copy of sort() and various other functions
> (see the arm-deps in [2]).
>
> So long as the stub is not built with ASAN, that should work.

 Thanks, this should help, as we already build the stub without ASAN 
 instrumentation.
>>>
>>> Indeed. I did not mention instrumentation in the commit log for those
>>> patches, but obviously, something like KASAN instrumentation cannot be
>>> tolerated in the stub since it makes assumptions about the memory
>>> layout
>>
>> I'll review your latest EFI stub isolation patches and try Kasan again
>> on top (most likely tomorrow).
> 
> You'd better wait for v7, because kasan patches will need some adjustment.
> Since stub is isolated,  we need to handle memcpy vs __memcpy stuff the same
> way as we do in x86. Now we also need to #undef memset/memcpy/memmove in ARM64
> (just like this was done for x86).
> 

Hm, I was wrong, we don't need that.

I thought the EFI stub isolation patches create a copy of mem*() functions in 
the stub,
but they are just create aliases with __efistub_ prefix.

We only need to create some more aliases for KASAN.
The following patch on top of the EFI stub isolation series works for me.


Signed-off-by: Andrey Ryabinin 
---
 arch/arm64/kernel/image.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
index e083af0..6eb8fee 100644
--- a/arch/arm64/kernel/image.h
+++ b/arch/arm64/kernel/image.h
@@ -80,6 +80,12 @@ __efistub_strcmp = __pi_strcmp;
 __efistub_strncmp  = __pi_strncmp;
 __efistub___flush_dcache_area  = __pi___flush_dcache_area;
 
+#ifdef CONFIG_KASAN
+__efistub___memcpy = __pi_memcpy;
+__efistub___memmove= __pi_memmove;
+__efistub___memset = __pi_memset;
+#endif
+
 __efistub__text= _text;
 __efistub__end = _end;
 __efistub__edata   = _edata;
-- 
2.4.9






--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Mark Rutland]

On Fri, Oct 09, 2015 at 12:32:18PM +0300, Andrey Ryabinin wrote:
[...]

> I thought the EFI stub isolation patches create a copy of mem*() functions in 
> the stub,
> but they are just create aliases with __efistub_ prefix.
> 
> We only need to create some more aliases for KASAN.
> The following patch on top of the EFI stub isolation series works for me.
> 
> 
> Signed-off-by: Andrey Ryabinin 
> ---
>  arch/arm64/kernel/image.h | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
> index e083af0..6eb8fee 100644
> --- a/arch/arm64/kernel/image.h
> +++ b/arch/arm64/kernel/image.h
> @@ -80,6 +80,12 @@ __efistub_strcmp   = __pi_strcmp;
>  __efistub_strncmp= __pi_strncmp;
>  __efistub___flush_dcache_area= __pi___flush_dcache_area;
>  
> +#ifdef CONFIG_KASAN
> +__efistub___memcpy   = __pi_memcpy;
> +__efistub___memmove  = __pi_memmove;
> +__efistub___memset   = __pi_memset;
> +#endif

Ard's v4 stub isolation series has these aliases [1], as the stub
requires these aliases regardless of KASAN in order to link.

Thanks,
Mark.

[1] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/375708.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Andrey Ryabinin]

2015-10-09 12:48 GMT+03:00 Mark Rutland :
> On Fri, Oct 09, 2015 at 12:32:18PM +0300, Andrey Ryabinin wrote:
> [...]
>
>> I thought the EFI stub isolation patches create a copy of mem*() functions 
>> in the stub,
>> but they are just create aliases with __efistub_ prefix.
>>
>> We only need to create some more aliases for KASAN.
>> The following patch on top of the EFI stub isolation series works for me.
>>
>>
>> Signed-off-by: Andrey Ryabinin 
>> ---
>>  arch/arm64/kernel/image.h | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
>> index e083af0..6eb8fee 100644
>> --- a/arch/arm64/kernel/image.h
>> +++ b/arch/arm64/kernel/image.h
>> @@ -80,6 +80,12 @@ __efistub_strcmp   = __pi_strcmp;
>>  __efistub_strncmp= __pi_strncmp;
>>  __efistub___flush_dcache_area= __pi___flush_dcache_area;
>>
>> +#ifdef CONFIG_KASAN
>> +__efistub___memcpy   = __pi_memcpy;
>> +__efistub___memmove  = __pi_memmove;
>> +__efistub___memset   = __pi_memset;
>> +#endif
>
> Ard's v4 stub isolation series has these aliases [1], as the stub
> requires these aliases regardless of KASAN in order to link.

Stub isolation series has __efistub_memcpy, not __efistub___memcpy
(two additional '_').
The thing is, KASAN provides own implementation of memcpy() which
checks memory before access.
The original 'memcpy()' becomes __memcpy(), so we could still use it.
In code that not instrumented by KASAN (like the EFI stub) we replace
KASAN's memcpy() with the original __mempcy():
#define memcpy() __memcpy()

So with CONFIG_KASAN=y the EFI stub uses __memcpy, thus we need to
create the __efistub___memcpy alias.

>
> Thanks,
> Mark.
>
> [1] 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/375708.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Mark Rutland]

On Fri, Oct 09, 2015 at 01:18:09PM +0300, Andrey Ryabinin wrote:
> 2015-10-09 12:48 GMT+03:00 Mark Rutland :
> > On Fri, Oct 09, 2015 at 12:32:18PM +0300, Andrey Ryabinin wrote:
> > [...]
> >
> >> I thought the EFI stub isolation patches create a copy of mem*() functions 
> >> in the stub,
> >> but they are just create aliases with __efistub_ prefix.
> >>
> >> We only need to create some more aliases for KASAN.
> >> The following patch on top of the EFI stub isolation series works for me.
> >>
> >>
> >> Signed-off-by: Andrey Ryabinin 
> >> ---
> >>  arch/arm64/kernel/image.h | 6 ++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
> >> index e083af0..6eb8fee 100644
> >> --- a/arch/arm64/kernel/image.h
> >> +++ b/arch/arm64/kernel/image.h
> >> @@ -80,6 +80,12 @@ __efistub_strcmp   = __pi_strcmp;
> >>  __efistub_strncmp= __pi_strncmp;
> >>  __efistub___flush_dcache_area= __pi___flush_dcache_area;
> >>
> >> +#ifdef CONFIG_KASAN
> >> +__efistub___memcpy   = __pi_memcpy;
> >> +__efistub___memmove  = __pi_memmove;
> >> +__efistub___memset   = __pi_memset;
> >> +#endif
> >
> > Ard's v4 stub isolation series has these aliases [1], as the stub
> > requires these aliases regardless of KASAN in order to link.
> 
> Stub isolation series has __efistub_memcpy, not __efistub___memcpy
> (two additional '_').

Ah, I see, sorry for my sloppy reading.

> The thing is, KASAN provides own implementation of memcpy() which
> checks memory before access.
> The original 'memcpy()' becomes __memcpy(), so we could still use it.

Ok.

> In code that not instrumented by KASAN (like the EFI stub) we replace
> KASAN's memcpy() with the original __mempcy():
> #define memcpy() __memcpy()

I'm a little confused by this. Surely that doesn't override implicit
calls generated by the compiler, leaving us with a mixture of calls to
memcpy and __memcpy?

That doesn't matter for the stub, as both __efistub_mem* and
__efistub___mem* would point at __pe_mem*, but doesn't that matter for
other users that shouldn't be instrumented?

Is that not a problem, or do we inhibit/override that somehow?

> So with CONFIG_KASAN=y the EFI stub uses __memcpy, thus we need to
> create the __efistub___memcpy alias.

Ok, that makes sense to me.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Andrey Ryabinin]

2015-10-09 15:42 GMT+03:00 Mark Rutland :
> On Fri, Oct 09, 2015 at 01:18:09PM +0300, Andrey Ryabinin wrote:
>> 2015-10-09 12:48 GMT+03:00 Mark Rutland :
>> > On Fri, Oct 09, 2015 at 12:32:18PM +0300, Andrey Ryabinin wrote:
>> > [...]
>> >
>> >> I thought the EFI stub isolation patches create a copy of mem*() 
>> >> functions in the stub,
>> >> but they are just create aliases with __efistub_ prefix.
>> >>
>> >> We only need to create some more aliases for KASAN.
>> >> The following patch on top of the EFI stub isolation series works for me.
>> >>
>> >>
>> >> Signed-off-by: Andrey Ryabinin 
>> >> ---
>> >>  arch/arm64/kernel/image.h | 6 ++
>> >>  1 file changed, 6 insertions(+)
>> >>
>> >> diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
>> >> index e083af0..6eb8fee 100644
>> >> --- a/arch/arm64/kernel/image.h
>> >> +++ b/arch/arm64/kernel/image.h
>> >> @@ -80,6 +80,12 @@ __efistub_strcmp   = __pi_strcmp;
>> >>  __efistub_strncmp= __pi_strncmp;
>> >>  __efistub___flush_dcache_area= __pi___flush_dcache_area;
>> >>
>> >> +#ifdef CONFIG_KASAN
>> >> +__efistub___memcpy   = __pi_memcpy;
>> >> +__efistub___memmove  = __pi_memmove;
>> >> +__efistub___memset   = __pi_memset;
>> >> +#endif
>> >
>> > Ard's v4 stub isolation series has these aliases [1], as the stub
>> > requires these aliases regardless of KASAN in order to link.
>>
>> Stub isolation series has __efistub_memcpy, not __efistub___memcpy
>> (two additional '_').
>
> Ah, I see, sorry for my sloppy reading.
>
>> The thing is, KASAN provides own implementation of memcpy() which
>> checks memory before access.
>> The original 'memcpy()' becomes __memcpy(), so we could still use it.
>
> Ok.
>
>> In code that not instrumented by KASAN (like the EFI stub) we replace
>> KASAN's memcpy() with the original __mempcy():
>> #define memcpy() __memcpy()
>
> I'm a little confused by this. Surely that doesn't override implicit
> calls generated by the compiler, leaving us with a mixture of calls to
> memcpy and __memcpy?
>
> That doesn't matter for the stub, as both __efistub_mem* and
> __efistub___mem* would point at __pe_mem*, but doesn't that matter for
> other users that shouldn't be instrumented?
>
> Is that not a problem, or do we inhibit/override that somehow?
>

You are right, GCC could emit memcpy() call. It's just not a problem so far.
The amount of not instrumented code is fairly small (some low-level
x86 code, kasan internals and slub allocator).

The purpose of these defines is to not spread kasan-specific details
across unrelated code.
E.g. there are a lot of memcpy()/memset() calls in slub that used to
access object's redzone or
freed objects. So it simpler to redefine memset, rather then somehow
mangle that code.

>> So with CONFIG_KASAN=y the EFI stub uses __memcpy, thus we need to
>> create the __efistub___memcpy alias.
>
> Ok, that makes sense to me.
>
> Thanks,
> Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Andrey Ryabinin]

2015-10-08 18:11 GMT+03:00 Catalin Marinas :
> On Thu, Oct 08, 2015 at 02:09:26PM +0200, Ard Biesheuvel wrote:
>> On 8 October 2015 at 13:23, Andrey Ryabinin  wrote:
>> > On 10/08/2015 02:11 PM, Mark Rutland wrote:
>> >> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
>> >>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
>>  On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
>> > As usual patches available in git
>> >   git://github.com/aryabinin/linux.git kasan/arm64v6
>> >
>> > Changes since v5:
>> >  - Rebase on top of 4.3-rc1
>> >  - Fixed EFI boot.
>> >  - Updated Doc/features/KASAN.
>> 
>>  I tried to merge these patches (apart from the x86 one which is already
>>  merged) but it still doesn't boot on Juno as an EFI application.
>> 
>> >>>
>> >>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
>> >>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
>> >>> regions")
>> >>> It introduced sort() call in efi_get_virtmap().
>> >>> sort() is generic kernel function and it's instrumented, so we crash
>> >>> when KASAN tries to access shadow in sort().
>> >>
>> >> I believe this is solved by Ard's stub isolation series [1,2], which
>> >> will build a stub-specific copy of sort() and various other functions
>> >> (see the arm-deps in [2]).
>> >>
>> >> So long as the stub is not built with ASAN, that should work.
>> >
>> > Thanks, this should help, as we already build the stub without ASAN 
>> > instrumentation.
>>
>> Indeed. I did not mention instrumentation in the commit log for those
>> patches, but obviously, something like KASAN instrumentation cannot be
>> tolerated in the stub since it makes assumptions about the memory
>> layout
>
> I'll review your latest EFI stub isolation patches and try Kasan again
> on top (most likely tomorrow).

You'd better wait for v7, because kasan patches will need some adjustment.
Since stub is isolated,  we need to handle memcpy vs __memcpy stuff the same
way as we do in x86. Now we also need to #undef memset/memcpy/memmove in ARM64
(just like this was done for x86).

But instead of spreading these #undef across various headers, I will
make a patch (most likely tomorrow)
which will get rid of these #undefs completely (the idea was described
here: https://lkml.org/lkml/2015/9/29/607)
And I'll will send v7 on top of that patch + Ard's work.


> Thanks.
>
> --
> Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Ard Biesheuvel]

(+ Matt)

On 8 October 2015 at 17:11, Catalin Marinas  wrote:
> On Thu, Oct 08, 2015 at 02:09:26PM +0200, Ard Biesheuvel wrote:
>> On 8 October 2015 at 13:23, Andrey Ryabinin  wrote:
>> > On 10/08/2015 02:11 PM, Mark Rutland wrote:
>> >> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
>> >>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
>>  On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
>> > As usual patches available in git
>> >   git://github.com/aryabinin/linux.git kasan/arm64v6
>> >
>> > Changes since v5:
>> >  - Rebase on top of 4.3-rc1
>> >  - Fixed EFI boot.
>> >  - Updated Doc/features/KASAN.
>> 
>>  I tried to merge these patches (apart from the x86 one which is already
>>  merged) but it still doesn't boot on Juno as an EFI application.
>> 
>> >>>
>> >>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
>> >>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
>> >>> regions")
>> >>> It introduced sort() call in efi_get_virtmap().
>> >>> sort() is generic kernel function and it's instrumented, so we crash
>> >>> when KASAN tries to access shadow in sort().
>> >>
>> >> I believe this is solved by Ard's stub isolation series [1,2], which
>> >> will build a stub-specific copy of sort() and various other functions
>> >> (see the arm-deps in [2]).
>> >>
>> >> So long as the stub is not built with ASAN, that should work.
>> >
>> > Thanks, this should help, as we already build the stub without ASAN 
>> > instrumentation.
>>
>> Indeed. I did not mention instrumentation in the commit log for those
>> patches, but obviously, something like KASAN instrumentation cannot be
>> tolerated in the stub since it makes assumptions about the memory
>> layout
>
> I'll review your latest EFI stub isolation patches and try Kasan again
> on top (most likely tomorrow).
>

OK.

If you (and Matt) are ok with those, I'd like to spin a new version
that only adds strcmp(). We need that in a separate series that only
touches libstub, so with strcmp() added, we are completely independent
in terms of merging order.

Thanks,
Ard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Catalin Marinas]

On Thu, Oct 08, 2015 at 02:09:26PM +0200, Ard Biesheuvel wrote:
> On 8 October 2015 at 13:23, Andrey Ryabinin  wrote:
> > On 10/08/2015 02:11 PM, Mark Rutland wrote:
> >> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
> >>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
>  On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
> > As usual patches available in git
> >   git://github.com/aryabinin/linux.git kasan/arm64v6
> >
> > Changes since v5:
> >  - Rebase on top of 4.3-rc1
> >  - Fixed EFI boot.
> >  - Updated Doc/features/KASAN.
> 
>  I tried to merge these patches (apart from the x86 one which is already
>  merged) but it still doesn't boot on Juno as an EFI application.
> 
> >>>
> >>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
> >>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
> >>> regions")
> >>> It introduced sort() call in efi_get_virtmap().
> >>> sort() is generic kernel function and it's instrumented, so we crash
> >>> when KASAN tries to access shadow in sort().
> >>
> >> I believe this is solved by Ard's stub isolation series [1,2], which
> >> will build a stub-specific copy of sort() and various other functions
> >> (see the arm-deps in [2]).
> >>
> >> So long as the stub is not built with ASAN, that should work.
> >
> > Thanks, this should help, as we already build the stub without ASAN 
> > instrumentation.
> 
> Indeed. I did not mention instrumentation in the commit log for those
> patches, but obviously, something like KASAN instrumentation cannot be
> tolerated in the stub since it makes assumptions about the memory
> layout

I'll review your latest EFI stub isolation patches and try Kasan again
on top (most likely tomorrow).

Thanks.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Ard Biesheuvel]

On 8 October 2015 at 13:23, Andrey Ryabinin  wrote:
> On 10/08/2015 02:11 PM, Mark Rutland wrote:
>> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
>>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
 On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
> As usual patches available in git
>   git://github.com/aryabinin/linux.git kasan/arm64v6
>
> Changes since v5:
>  - Rebase on top of 4.3-rc1
>  - Fixed EFI boot.
>  - Updated Doc/features/KASAN.

 I tried to merge these patches (apart from the x86 one which is already
 merged) but it still doesn't boot on Juno as an EFI application.

>>>
>>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
>>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
>>> regions")
>>> It introduced sort() call in efi_get_virtmap().
>>> sort() is generic kernel function and it's instrumented, so we crash
>>> when KASAN tries to access shadow in sort().
>>
>> I believe this is solved by Ard's stub isolation series [1,2], which
>> will build a stub-specific copy of sort() and various other functions
>> (see the arm-deps in [2]).
>>
>> So long as the stub is not built with ASAN, that should work.
>
> Thanks, this should help, as we already build the stub without ASAN 
> instrumentation.
>

Indeed. I did not mention instrumentation in the commit log for those
patches, but obviously, something like KASAN instrumentation cannot be
tolerated in the stub since it makes assumptions about the memory
layout
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Andrey Ryabinin]

On 10/08/2015 02:11 PM, Mark Rutland wrote:
> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
>>> On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
 As usual patches available in git
   git://github.com/aryabinin/linux.git kasan/arm64v6

 Changes since v5:
  - Rebase on top of 4.3-rc1
  - Fixed EFI boot.
  - Updated Doc/features/KASAN.
>>>
>>> I tried to merge these patches (apart from the x86 one which is already
>>> merged) but it still doesn't boot on Juno as an EFI application.
>>>
>>
>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
>> regions")
>> It introduced sort() call in efi_get_virtmap().
>> sort() is generic kernel function and it's instrumented, so we crash
>> when KASAN tries to access shadow in sort().
> 
> I believe this is solved by Ard's stub isolation series [1,2], which
> will build a stub-specific copy of sort() and various other functions
> (see the arm-deps in [2]).
> 
> So long as the stub is not built with ASAN, that should work.

Thanks, this should help, as we already build the stub without ASAN 
instrumentation.

> 
> Mark.
> 
> [1] 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/373807.html
> [2] 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/373808.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Mark Rutland]

On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
> > On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
> >> As usual patches available in git
> >>   git://github.com/aryabinin/linux.git kasan/arm64v6
> >>
> >> Changes since v5:
> >>  - Rebase on top of 4.3-rc1
> >>  - Fixed EFI boot.
> >>  - Updated Doc/features/KASAN.
> >
> > I tried to merge these patches (apart from the x86 one which is already
> > merged) but it still doesn't boot on Juno as an EFI application.
> >
> 
> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
> regions")
> It introduced sort() call in efi_get_virtmap().
> sort() is generic kernel function and it's instrumented, so we crash
> when KASAN tries to access shadow in sort().

I believe this is solved by Ard's stub isolation series [1,2], which
will build a stub-specific copy of sort() and various other functions
(see the arm-deps in [2]).

So long as the stub is not built with ASAN, that should work.

Mark.

[1] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/373807.html
[2] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/373808.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Mark Rutland]

On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
> > On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
> >> As usual patches available in git
> >>   git://github.com/aryabinin/linux.git kasan/arm64v6
> >>
> >> Changes since v5:
> >>  - Rebase on top of 4.3-rc1
> >>  - Fixed EFI boot.
> >>  - Updated Doc/features/KASAN.
> >
> > I tried to merge these patches (apart from the x86 one which is already
> > merged) but it still doesn't boot on Juno as an EFI application.
> >
> 
> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
> regions")
> It introduced sort() call in efi_get_virtmap().
> sort() is generic kernel function and it's instrumented, so we crash
> when KASAN tries to access shadow in sort().
> 
> [+CC efi some guys]
> 
> Comment in drivers/firmware/efi/libstub/Makefile says that EFI stub
> executes with MMU disabled:
> # The stub may be linked into the kernel proper or into a separate
> boot binary,
> # but in either case, it executes before the kernel does (with MMU
> disabled) so
> # things like ftrace and stack-protector are likely to cause trouble if 
> left
> # enabled, even if doing so doesn't break the build.
> 
> But in arch/arm64/kernel/efi-entry.S:
> * We arrive here from the EFI boot manager with:
> *
> ** CPU in little-endian mode
> ** MMU on with identity-mapped RAM
> 
> So is MMU enabled in ARM64 efi-stub?

The stub is executed as an EFI application, which means that the MMU is
on, and the page tables are an idmap owned by the EFI implementation.

> If yes, we could solve this issue by mapping KASAN early shadow in efi stub.

As the page tables are owned by the implemenation and not the kernel, we
cannot alter them (at least not until we've called ExitBootServices(),
which happens relatively late).

Can we not build the stub without ASAN protections?

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Andrey Ryabinin]

2015-10-07 13:04 GMT+03:00 Catalin Marinas :
> On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
>> As usual patches available in git
>>   git://github.com/aryabinin/linux.git kasan/arm64v6
>>
>> Changes since v5:
>>  - Rebase on top of 4.3-rc1
>>  - Fixed EFI boot.
>>  - Updated Doc/features/KASAN.
>
> I tried to merge these patches (apart from the x86 one which is already
> merged) but it still doesn't boot on Juno as an EFI application.
>

4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
regions")
It introduced sort() call in efi_get_virtmap().
sort() is generic kernel function and it's instrumented, so we crash
when KASAN tries to access shadow in sort().

[+CC efi some guys]

Comment in drivers/firmware/efi/libstub/Makefile says that EFI stub
executes with MMU disabled:
# The stub may be linked into the kernel proper or into a separate
boot binary,
# but in either case, it executes before the kernel does (with MMU
disabled) so
# things like ftrace and stack-protector are likely to cause trouble if left
# enabled, even if doing so doesn't break the build.

But in arch/arm64/kernel/efi-entry.S:
* We arrive here from the EFI boot manager with:
*
** CPU in little-endian mode
** MMU on with identity-mapped RAM

So is MMU enabled in ARM64 efi-stub?
If yes, we could solve this issue by mapping KASAN early shadow in efi stub.

> --
> Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Mark Rutland]

On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
> > On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
> >> As usual patches available in git
> >>   git://github.com/aryabinin/linux.git kasan/arm64v6
> >>
> >> Changes since v5:
> >>  - Rebase on top of 4.3-rc1
> >>  - Fixed EFI boot.
> >>  - Updated Doc/features/KASAN.
> >
> > I tried to merge these patches (apart from the x86 one which is already
> > merged) but it still doesn't boot on Juno as an EFI application.
> >
> 
> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
> regions")
> It introduced sort() call in efi_get_virtmap().
> sort() is generic kernel function and it's instrumented, so we crash
> when KASAN tries to access shadow in sort().
> 
> [+CC efi some guys]
> 
> Comment in drivers/firmware/efi/libstub/Makefile says that EFI stub
> executes with MMU disabled:
> # The stub may be linked into the kernel proper or into a separate
> boot binary,
> # but in either case, it executes before the kernel does (with MMU
> disabled) so
> # things like ftrace and stack-protector are likely to cause trouble if 
> left
> # enabled, even if doing so doesn't break the build.
> 
> But in arch/arm64/kernel/efi-entry.S:
> * We arrive here from the EFI boot manager with:
> *
> ** CPU in little-endian mode
> ** MMU on with identity-mapped RAM
> 
> So is MMU enabled in ARM64 efi-stub?

The stub is executed as an EFI application, which means that the MMU is
on, and the page tables are an idmap owned by the EFI implementation.

> If yes, we could solve this issue by mapping KASAN early shadow in efi stub.

As the page tables are owned by the implemenation and not the kernel, we
cannot alter them (at least not until we've called ExitBootServices(),
which happens relatively late).

Can we not build the stub without ASAN protections?

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Andrey Ryabinin]

On 10/08/2015 02:11 PM, Mark Rutland wrote:
> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
>>> On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
 As usual patches available in git
   git://github.com/aryabinin/linux.git kasan/arm64v6

 Changes since v5:
  - Rebase on top of 4.3-rc1
  - Fixed EFI boot.
  - Updated Doc/features/KASAN.
>>>
>>> I tried to merge these patches (apart from the x86 one which is already
>>> merged) but it still doesn't boot on Juno as an EFI application.
>>>
>>
>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
>> regions")
>> It introduced sort() call in efi_get_virtmap().
>> sort() is generic kernel function and it's instrumented, so we crash
>> when KASAN tries to access shadow in sort().
> 
> I believe this is solved by Ard's stub isolation series [1,2], which
> will build a stub-specific copy of sort() and various other functions
> (see the arm-deps in [2]).
> 
> So long as the stub is not built with ASAN, that should work.

Thanks, this should help, as we already build the stub without ASAN 
instrumentation.

> 
> Mark.
> 
> [1] 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/373807.html
> [2] 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/373808.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Ard Biesheuvel]

On 8 October 2015 at 13:23, Andrey Ryabinin  wrote:
> On 10/08/2015 02:11 PM, Mark Rutland wrote:
>> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
>>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
 On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
> As usual patches available in git
>   git://github.com/aryabinin/linux.git kasan/arm64v6
>
> Changes since v5:
>  - Rebase on top of 4.3-rc1
>  - Fixed EFI boot.
>  - Updated Doc/features/KASAN.

 I tried to merge these patches (apart from the x86 one which is already
 merged) but it still doesn't boot on Juno as an EFI application.

>>>
>>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
>>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
>>> regions")
>>> It introduced sort() call in efi_get_virtmap().
>>> sort() is generic kernel function and it's instrumented, so we crash
>>> when KASAN tries to access shadow in sort().
>>
>> I believe this is solved by Ard's stub isolation series [1,2], which
>> will build a stub-specific copy of sort() and various other functions
>> (see the arm-deps in [2]).
>>
>> So long as the stub is not built with ASAN, that should work.
>
> Thanks, this should help, as we already build the stub without ASAN 
> instrumentation.
>

Indeed. I did not mention instrumentation in the commit log for those
patches, but obviously, something like KASAN instrumentation cannot be
tolerated in the stub since it makes assumptions about the memory
layout
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Andrey Ryabinin]

2015-10-07 13:04 GMT+03:00 Catalin Marinas :
> On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
>> As usual patches available in git
>>   git://github.com/aryabinin/linux.git kasan/arm64v6
>>
>> Changes since v5:
>>  - Rebase on top of 4.3-rc1
>>  - Fixed EFI boot.
>>  - Updated Doc/features/KASAN.
>
> I tried to merge these patches (apart from the x86 one which is already
> merged) but it still doesn't boot on Juno as an EFI application.
>

4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
regions")
It introduced sort() call in efi_get_virtmap().
sort() is generic kernel function and it's instrumented, so we crash
when KASAN tries to access shadow in sort().

[+CC efi some guys]

Comment in drivers/firmware/efi/libstub/Makefile says that EFI stub
executes with MMU disabled:
# The stub may be linked into the kernel proper or into a separate
boot binary,
# but in either case, it executes before the kernel does (with MMU
disabled) so
# things like ftrace and stack-protector are likely to cause trouble if left
# enabled, even if doing so doesn't break the build.

But in arch/arm64/kernel/efi-entry.S:
* We arrive here from the EFI boot manager with:
*
** CPU in little-endian mode
** MMU on with identity-mapped RAM

So is MMU enabled in ARM64 efi-stub?
If yes, we could solve this issue by mapping KASAN early shadow in efi stub.

> --
> Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Mark Rutland]

On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
> > On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
> >> As usual patches available in git
> >>   git://github.com/aryabinin/linux.git kasan/arm64v6
> >>
> >> Changes since v5:
> >>  - Rebase on top of 4.3-rc1
> >>  - Fixed EFI boot.
> >>  - Updated Doc/features/KASAN.
> >
> > I tried to merge these patches (apart from the x86 one which is already
> > merged) but it still doesn't boot on Juno as an EFI application.
> >
> 
> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
> regions")
> It introduced sort() call in efi_get_virtmap().
> sort() is generic kernel function and it's instrumented, so we crash
> when KASAN tries to access shadow in sort().

I believe this is solved by Ard's stub isolation series [1,2], which
will build a stub-specific copy of sort() and various other functions
(see the arm-deps in [2]).

So long as the stub is not built with ASAN, that should work.

Mark.

[1] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/373807.html
[2] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/373808.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Catalin Marinas]

On Thu, Oct 08, 2015 at 02:09:26PM +0200, Ard Biesheuvel wrote:
> On 8 October 2015 at 13:23, Andrey Ryabinin  wrote:
> > On 10/08/2015 02:11 PM, Mark Rutland wrote:
> >> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
> >>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
>  On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
> > As usual patches available in git
> >   git://github.com/aryabinin/linux.git kasan/arm64v6
> >
> > Changes since v5:
> >  - Rebase on top of 4.3-rc1
> >  - Fixed EFI boot.
> >  - Updated Doc/features/KASAN.
> 
>  I tried to merge these patches (apart from the x86 one which is already
>  merged) but it still doesn't boot on Juno as an EFI application.
> 
> >>>
> >>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
> >>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
> >>> regions")
> >>> It introduced sort() call in efi_get_virtmap().
> >>> sort() is generic kernel function and it's instrumented, so we crash
> >>> when KASAN tries to access shadow in sort().
> >>
> >> I believe this is solved by Ard's stub isolation series [1,2], which
> >> will build a stub-specific copy of sort() and various other functions
> >> (see the arm-deps in [2]).
> >>
> >> So long as the stub is not built with ASAN, that should work.
> >
> > Thanks, this should help, as we already build the stub without ASAN 
> > instrumentation.
> 
> Indeed. I did not mention instrumentation in the commit log for those
> patches, but obviously, something like KASAN instrumentation cannot be
> tolerated in the stub since it makes assumptions about the memory
> layout

I'll review your latest EFI stub isolation patches and try Kasan again
on top (most likely tomorrow).

Thanks.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Ard Biesheuvel]

(+ Matt)

On 8 October 2015 at 17:11, Catalin Marinas  wrote:
> On Thu, Oct 08, 2015 at 02:09:26PM +0200, Ard Biesheuvel wrote:
>> On 8 October 2015 at 13:23, Andrey Ryabinin  wrote:
>> > On 10/08/2015 02:11 PM, Mark Rutland wrote:
>> >> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
>> >>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
>>  On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
>> > As usual patches available in git
>> >   git://github.com/aryabinin/linux.git kasan/arm64v6
>> >
>> > Changes since v5:
>> >  - Rebase on top of 4.3-rc1
>> >  - Fixed EFI boot.
>> >  - Updated Doc/features/KASAN.
>> 
>>  I tried to merge these patches (apart from the x86 one which is already
>>  merged) but it still doesn't boot on Juno as an EFI application.
>> 
>> >>>
>> >>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
>> >>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
>> >>> regions")
>> >>> It introduced sort() call in efi_get_virtmap().
>> >>> sort() is generic kernel function and it's instrumented, so we crash
>> >>> when KASAN tries to access shadow in sort().
>> >>
>> >> I believe this is solved by Ard's stub isolation series [1,2], which
>> >> will build a stub-specific copy of sort() and various other functions
>> >> (see the arm-deps in [2]).
>> >>
>> >> So long as the stub is not built with ASAN, that should work.
>> >
>> > Thanks, this should help, as we already build the stub without ASAN 
>> > instrumentation.
>>
>> Indeed. I did not mention instrumentation in the commit log for those
>> patches, but obviously, something like KASAN instrumentation cannot be
>> tolerated in the stub since it makes assumptions about the memory
>> layout
>
> I'll review your latest EFI stub isolation patches and try Kasan again
> on top (most likely tomorrow).
>

OK.

If you (and Matt) are ok with those, I'd like to spin a new version
that only adds strcmp(). We need that in a separate series that only
touches libstub, so with strcmp() added, we are completely independent
in terms of merging order.

Thanks,
Ard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Andrey Ryabinin]

2015-10-08 18:11 GMT+03:00 Catalin Marinas :
> On Thu, Oct 08, 2015 at 02:09:26PM +0200, Ard Biesheuvel wrote:
>> On 8 October 2015 at 13:23, Andrey Ryabinin  wrote:
>> > On 10/08/2015 02:11 PM, Mark Rutland wrote:
>> >> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
>> >>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
>>  On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
>> > As usual patches available in git
>> >   git://github.com/aryabinin/linux.git kasan/arm64v6
>> >
>> > Changes since v5:
>> >  - Rebase on top of 4.3-rc1
>> >  - Fixed EFI boot.
>> >  - Updated Doc/features/KASAN.
>> 
>>  I tried to merge these patches (apart from the x86 one which is already
>>  merged) but it still doesn't boot on Juno as an EFI application.
>> 
>> >>>
>> >>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
>> >>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
>> >>> regions")
>> >>> It introduced sort() call in efi_get_virtmap().
>> >>> sort() is generic kernel function and it's instrumented, so we crash
>> >>> when KASAN tries to access shadow in sort().
>> >>
>> >> I believe this is solved by Ard's stub isolation series [1,2], which
>> >> will build a stub-specific copy of sort() and various other functions
>> >> (see the arm-deps in [2]).
>> >>
>> >> So long as the stub is not built with ASAN, that should work.
>> >
>> > Thanks, this should help, as we already build the stub without ASAN 
>> > instrumentation.
>>
>> Indeed. I did not mention instrumentation in the commit log for those
>> patches, but obviously, something like KASAN instrumentation cannot be
>> tolerated in the stub since it makes assumptions about the memory
>> layout
>
> I'll review your latest EFI stub isolation patches and try Kasan again
> on top (most likely tomorrow).

You'd better wait for v7, because kasan patches will need some adjustment.
Since stub is isolated,  we need to handle memcpy vs __memcpy stuff the same
way as we do in x86. Now we also need to #undef memset/memcpy/memmove in ARM64
(just like this was done for x86).

But instead of spreading these #undef across various headers, I will
make a patch (most likely tomorrow)
which will get rid of these #undefs completely (the idea was described
here: https://lkml.org/lkml/2015/9/29/607)
And I'll will send v7 on top of that patch + Ard's work.


> Thanks.
>
> --
> Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Catalin Marinas]

On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
> As usual patches available in git
>   git://github.com/aryabinin/linux.git kasan/arm64v6
> 
> Changes since v5:
>  - Rebase on top of 4.3-rc1
>  - Fixed EFI boot.
>  - Updated Doc/features/KASAN.

I tried to merge these patches (apart from the x86 one which is already
merged) but it still doesn't boot on Juno as an EFI application.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 0/6] KASAN for arm64

[Catalin Marinas]

On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
> As usual patches available in git
>   git://github.com/aryabinin/linux.git kasan/arm64v6
> 
> Changes since v5:
>  - Rebase on top of 4.3-rc1
>  - Fixed EFI boot.
>  - Updated Doc/features/KASAN.

I tried to merge these patches (apart from the x86 one which is already
merged) but it still doesn't boot on Juno as an EFI application.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/