[PATCH] mm/kasan: map KASAN zero page read only

[PATCH] mm/kasan: map KASAN zero page read only

[Ard Biesheuvel]

The original x86_64-only version of KASAN mapped its zero page
read-only, but this got lost when the code was generalised and
ported to arm64, since, at the time, the PAGE_KERNEL_RO define
did not exist. It has been added to arm64 in the mean time, so
let's use it.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 mm/kasan/kasan_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/kasan/kasan_init.c b/mm/kasan/kasan_init.c
index 3f9a41cf0ac6..8726a92604ad 100644
--- a/mm/kasan/kasan_init.c
+++ b/mm/kasan/kasan_init.c
@@ -49,7 +49,7 @@ static void __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
 	pte_t *pte = pte_offset_kernel(pmd, addr);
 	pte_t zero_pte;
 
-	zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL);
+	zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL_RO);
 	zero_pte = pte_wrprotect(zero_pte);
 
 	while (addr + PAGE_SIZE <= end) {
-- 
2.5.0

--
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>

[PATCH] mm/kasan: map KASAN zero page read only

[Ard Biesheuvel]

The original x86_64-only version of KASAN mapped its zero page
read-only, but this got lost when the code was generalised and
ported to arm64, since, at the time, the PAGE_KERNEL_RO define
did not exist. It has been added to arm64 in the mean time, so
let's use it.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 mm/kasan/kasan_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/kasan/kasan_init.c b/mm/kasan/kasan_init.c
index 3f9a41cf0ac6..8726a92604ad 100644
--- a/mm/kasan/kasan_init.c
+++ b/mm/kasan/kasan_init.c
@@ -49,7 +49,7 @@ static void __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
 	pte_t *pte = pte_offset_kernel(pmd, addr);
 	pte_t zero_pte;
 
-	zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL);
+	zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL_RO);
 	zero_pte = pte_wrprotect(zero_pte);
 
 	while (addr + PAGE_SIZE <= end) {
-- 
2.5.0

Re: [PATCH] mm/kasan: map KASAN zero page read only

[Andrey Ryabinin]

2016-01-06 18:54 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> The original x86_64-only version of KASAN mapped its zero page
> read-only, but this got lost when the code was generalised and
> ported to arm64, since, at the time, the PAGE_KERNEL_RO define
> did not exist. It has been added to arm64 in the mean time, so
> let's use it.
>

Read-only wasn't lost. Just look at the next line:
     zero_pte = pte_wrprotect(zero_pte);

PAGE_KERNEL_RO is not available on all architectures, thus it would be better
to not use it in generic code.


> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  mm/kasan/kasan_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/kasan/kasan_init.c b/mm/kasan/kasan_init.c
> index 3f9a41cf0ac6..8726a92604ad 100644
> --- a/mm/kasan/kasan_init.c
> +++ b/mm/kasan/kasan_init.c
> @@ -49,7 +49,7 @@ static void __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
>         pte_t *pte = pte_offset_kernel(pmd, addr);
>         pte_t zero_pte;
>
> -       zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL);
> +       zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL_RO);
>         zero_pte = pte_wrprotect(zero_pte);
>
>         while (addr + PAGE_SIZE <= end) {
> --
> 2.5.0
>

--
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>

[PATCH] mm/kasan: map KASAN zero page read only

[Andrey Ryabinin]

2016-01-06 18:54 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> The original x86_64-only version of KASAN mapped its zero page
> read-only, but this got lost when the code was generalised and
> ported to arm64, since, at the time, the PAGE_KERNEL_RO define
> did not exist. It has been added to arm64 in the mean time, so
> let's use it.
>

Read-only wasn't lost. Just look at the next line:
     zero_pte = pte_wrprotect(zero_pte);

PAGE_KERNEL_RO is not available on all architectures, thus it would be better
to not use it in generic code.


> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  mm/kasan/kasan_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/kasan/kasan_init.c b/mm/kasan/kasan_init.c
> index 3f9a41cf0ac6..8726a92604ad 100644
> --- a/mm/kasan/kasan_init.c
> +++ b/mm/kasan/kasan_init.c
> @@ -49,7 +49,7 @@ static void __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
>         pte_t *pte = pte_offset_kernel(pmd, addr);
>         pte_t zero_pte;
>
> -       zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL);
> +       zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL_RO);
>         zero_pte = pte_wrprotect(zero_pte);
>
>         while (addr + PAGE_SIZE <= end) {
> --
> 2.5.0
>

Re: [PATCH] mm/kasan: map KASAN zero page read only

[Ard Biesheuvel]

On 6 January 2016 at 20:48, Andrey Ryabinin <ryabinin.a.a@gmail.com> wrote:
> 2016-01-06 18:54 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>> The original x86_64-only version of KASAN mapped its zero page
>> read-only, but this got lost when the code was generalised and
>> ported to arm64, since, at the time, the PAGE_KERNEL_RO define
>> did not exist. It has been added to arm64 in the mean time, so
>> let's use it.
>>
>
> Read-only wasn't lost. Just look at the next line:
>      zero_pte = pte_wrprotect(zero_pte);
>
> PAGE_KERNEL_RO is not available on all architectures, thus it would be better
> to not use it in generic code.
>

OK, I didn't see that. For some reason, it is not working for me on
arm64, though.
I will investigate.

-- 
Ard.


>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  mm/kasan/kasan_init.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/kasan/kasan_init.c b/mm/kasan/kasan_init.c
>> index 3f9a41cf0ac6..8726a92604ad 100644
>> --- a/mm/kasan/kasan_init.c
>> +++ b/mm/kasan/kasan_init.c
>> @@ -49,7 +49,7 @@ static void __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
>>         pte_t *pte = pte_offset_kernel(pmd, addr);
>>         pte_t zero_pte;
>>
>> -       zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL);
>> +       zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL_RO);
>>         zero_pte = pte_wrprotect(zero_pte);
>>
>>         while (addr + PAGE_SIZE <= end) {
>> --
>> 2.5.0
>>

--
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>

[PATCH] mm/kasan: map KASAN zero page read only

[Ard Biesheuvel]

On 6 January 2016 at 20:48, Andrey Ryabinin <ryabinin.a.a@gmail.com> wrote:
> 2016-01-06 18:54 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>> The original x86_64-only version of KASAN mapped its zero page
>> read-only, but this got lost when the code was generalised and
>> ported to arm64, since, at the time, the PAGE_KERNEL_RO define
>> did not exist. It has been added to arm64 in the mean time, so
>> let's use it.
>>
>
> Read-only wasn't lost. Just look at the next line:
>      zero_pte = pte_wrprotect(zero_pte);
>
> PAGE_KERNEL_RO is not available on all architectures, thus it would be better
> to not use it in generic code.
>

OK, I didn't see that. For some reason, it is not working for me on
arm64, though.
I will investigate.

-- 
Ard.


>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  mm/kasan/kasan_init.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/kasan/kasan_init.c b/mm/kasan/kasan_init.c
>> index 3f9a41cf0ac6..8726a92604ad 100644
>> --- a/mm/kasan/kasan_init.c
>> +++ b/mm/kasan/kasan_init.c
>> @@ -49,7 +49,7 @@ static void __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
>>         pte_t *pte = pte_offset_kernel(pmd, addr);
>>         pte_t zero_pte;
>>
>> -       zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL);
>> +       zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL_RO);
>>         zero_pte = pte_wrprotect(zero_pte);
>>
>>         while (addr + PAGE_SIZE <= end) {
>> --
>> 2.5.0
>>

Re: [PATCH] mm/kasan: map KASAN zero page read only

[Catalin Marinas]

On Wed, Jan 06, 2016 at 09:18:03PM +0100, Ard Biesheuvel wrote:
> On 6 January 2016 at 20:48, Andrey Ryabinin <ryabinin.a.a@gmail.com> wrote:
> > 2016-01-06 18:54 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> >> The original x86_64-only version of KASAN mapped its zero page
> >> read-only, but this got lost when the code was generalised and
> >> ported to arm64, since, at the time, the PAGE_KERNEL_RO define
> >> did not exist. It has been added to arm64 in the mean time, so
> >> let's use it.
> >>
> >
> > Read-only wasn't lost. Just look at the next line:
> >      zero_pte = pte_wrprotect(zero_pte);
> >
> > PAGE_KERNEL_RO is not available on all architectures, thus it would be better
> > to not use it in generic code.
> 
> OK, I didn't see that. For some reason, it is not working for me on
> arm64, though.

It's because the arm64 set_pte_at() doesn't bother checking for
!PTE_WRITE to set PTE_RDONLY when mapping kernel pages. It works fine
for user though. That's because usually all read-only kernel mappings
already have PTE_RDONLY set via PAGE_KERNEL_RO.

We may need to change the set_pte_at logic a bit to cover the above
case.

-- 
Catalin

--
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>

[PATCH] mm/kasan: map KASAN zero page read only

[Catalin Marinas]

On Wed, Jan 06, 2016 at 09:18:03PM +0100, Ard Biesheuvel wrote:
> On 6 January 2016 at 20:48, Andrey Ryabinin <ryabinin.a.a@gmail.com> wrote:
> > 2016-01-06 18:54 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> >> The original x86_64-only version of KASAN mapped its zero page
> >> read-only, but this got lost when the code was generalised and
> >> ported to arm64, since, at the time, the PAGE_KERNEL_RO define
> >> did not exist. It has been added to arm64 in the mean time, so
> >> let's use it.
> >>
> >
> > Read-only wasn't lost. Just look at the next line:
> >      zero_pte = pte_wrprotect(zero_pte);
> >
> > PAGE_KERNEL_RO is not available on all architectures, thus it would be better
> > to not use it in generic code.
> 
> OK, I didn't see that. For some reason, it is not working for me on
> arm64, though.

It's because the arm64 set_pte_at() doesn't bother checking for
!PTE_WRITE to set PTE_RDONLY when mapping kernel pages. It works fine
for user though. That's because usually all read-only kernel mappings
already have PTE_RDONLY set via PAGE_KERNEL_RO.

We may need to change the set_pte_at logic a bit to cover the above
case.

-- 
Catalin

Re: [PATCH] mm/kasan: map KASAN zero page read only

[Ard Biesheuvel]

On 7 January 2016 at 10:51, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Wed, Jan 06, 2016 at 09:18:03PM +0100, Ard Biesheuvel wrote:
>> On 6 January 2016 at 20:48, Andrey Ryabinin <ryabinin.a.a@gmail.com> wrote:
>> > 2016-01-06 18:54 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>> >> The original x86_64-only version of KASAN mapped its zero page
>> >> read-only, but this got lost when the code was generalised and
>> >> ported to arm64, since, at the time, the PAGE_KERNEL_RO define
>> >> did not exist. It has been added to arm64 in the mean time, so
>> >> let's use it.
>> >>
>> >
>> > Read-only wasn't lost. Just look at the next line:
>> >      zero_pte = pte_wrprotect(zero_pte);
>> >
>> > PAGE_KERNEL_RO is not available on all architectures, thus it would be better
>> > to not use it in generic code.
>>
>> OK, I didn't see that. For some reason, it is not working for me on
>> arm64, though.
>
> It's because the arm64 set_pte_at() doesn't bother checking for
> !PTE_WRITE to set PTE_RDONLY when mapping kernel pages. It works fine
> for user though. That's because usually all read-only kernel mappings
> already have PTE_RDONLY set via PAGE_KERNEL_RO.
>
> We may need to change the set_pte_at logic a bit to cover the above
> case.
>

Yes, that would be useful. I had an interesting dive down a rabbit
hole yesterday due to the fact that the kasan zero page (which backs a
substantial chunk of the shadow area) was getting written to by one
mapping, and reporting KAsan errors via another.

-- 
Ard.

--
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>

[PATCH] mm/kasan: map KASAN zero page read only

[Ard Biesheuvel]

On 7 January 2016 at 10:51, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Wed, Jan 06, 2016 at 09:18:03PM +0100, Ard Biesheuvel wrote:
>> On 6 January 2016 at 20:48, Andrey Ryabinin <ryabinin.a.a@gmail.com> wrote:
>> > 2016-01-06 18:54 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>> >> The original x86_64-only version of KASAN mapped its zero page
>> >> read-only, but this got lost when the code was generalised and
>> >> ported to arm64, since, at the time, the PAGE_KERNEL_RO define
>> >> did not exist. It has been added to arm64 in the mean time, so
>> >> let's use it.
>> >>
>> >
>> > Read-only wasn't lost. Just look at the next line:
>> >      zero_pte = pte_wrprotect(zero_pte);
>> >
>> > PAGE_KERNEL_RO is not available on all architectures, thus it would be better
>> > to not use it in generic code.
>>
>> OK, I didn't see that. For some reason, it is not working for me on
>> arm64, though.
>
> It's because the arm64 set_pte_at() doesn't bother checking for
> !PTE_WRITE to set PTE_RDONLY when mapping kernel pages. It works fine
> for user though. That's because usually all read-only kernel mappings
> already have PTE_RDONLY set via PAGE_KERNEL_RO.
>
> We may need to change the set_pte_at logic a bit to cover the above
> case.
>

Yes, that would be useful. I had an interesting dive down a rabbit
hole yesterday due to the fact that the kasan zero page (which backs a
substantial chunk of the shadow area) was getting written to by one
mapping, and reporting KAsan errors via another.

-- 
Ard.