diff mbox

mm/kasan: map KASAN zero page read only

Message ID 1452095687-18136-1-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel Jan. 6, 2016, 3:54 p.m. UTC
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(-)

-- 
2.5.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Comments

Ard Biesheuvel Jan. 6, 2016, 8:18 p.m. UTC | #1
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

>>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Catalin Marinas Jan. 7, 2016, 9:51 a.m. UTC | #2
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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ard Biesheuvel Jan. 7, 2016, 10:01 a.m. UTC | #3
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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox

Patch

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