Currently, set_pte_at() only checks the software PTE_WRITE bit for user
mappings when it sets or clears the hardware PTE_RDONLY accordingly. The
kernel ptes are written directly without any modification, relying
solely on the protection bits in macros like PAGE_KERNEL. However,
modifying kernel pte attributes via pte_wrprotect() would be ignored by
set_pte_at(). Since pte_wrprotect() does not set PTE_RDONLY (it only
clears PTE_WRITE), the new permission is not taken into account.
This patch changes set_pte_at() to adjust the read-only permission for
kernel ptes as well. As a side effect, existing PROT_* definitions used
for kernel ioremap*() need to include PTE_DIRTY | PTE_WRITE.
(additionally, white space fix for PTE_KERNEL_ROX)
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reported-by: Ard Biesheuvel <redacted>
Cc: Will Deacon <redacted>
---
arch/arm64/include/asm/pgtable.h | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
(+ Andrey)
On 7 January 2016 at 17:07, Catalin Marinas [off-list ref] wrote:
quoted hunk
Currently, set_pte_at() only checks the software PTE_WRITE bit for user
mappings when it sets or clears the hardware PTE_RDONLY accordingly. The
kernel ptes are written directly without any modification, relying
solely on the protection bits in macros like PAGE_KERNEL. However,
modifying kernel pte attributes via pte_wrprotect() would be ignored by
set_pte_at(). Since pte_wrprotect() does not set PTE_RDONLY (it only
clears PTE_WRITE), the new permission is not taken into account.
This patch changes set_pte_at() to adjust the read-only permission for
kernel ptes as well. As a side effect, existing PROT_* definitions used
for kernel ioremap*() need to include PTE_DIRTY | PTE_WRITE.
(additionally, white space fix for PTE_KERNEL_ROX)
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reported-by: Ard Biesheuvel <redacted>
Cc: Will Deacon <redacted>
---
arch/arm64/include/asm/pgtable.h | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
This works, as far as I can tell. However, I still need the patch
below to make sure that the KAsan zero page is mapped read-only. (The
reason is that, depending on the alignment of the regions,
kasan_populate_zero_shadow() may never call
zero_[pud|pmd|pte]_populate())
Before this patch (and my change), the KAsan shadow regions looks like this:
0xffffff8000000000-0xffffff8200800000 8200M RW NX SHD AF UXN MEM/NORMAL
0xffffff8200800000-0xffffff8200c00000 4M RW NX SHD AF BLK UXN MEM/NORMAL
0xffffff8200c00000-0xffffff8800000000 24564M RW NX SHD AF UXN MEM/NORMAL
0xffffff8800000000-0xffffff8820200000 514M RW NX SHD AF BLK UXN MEM/NORMAL
and after:
0xffffff8000000000-0xffffff8200800000 8200M ro NX SHD AF UXN MEM/NORMAL
0xffffff8200800000-0xffffff8200c00000 4M RW NX SHD AF BLK UXN MEM/NORMAL
0xffffff8200c00000-0xffffff8800000000 24564M ro NX SHD AF UXN MEM/NORMAL
0xffffff8800000000-0xffffff8820200000 514M RW NX SHD AF BLK UXN MEM/NORMAL
---------8<--------------
directly, so we
+ * should make sure that it maps the zero page read-only.
+ */
+ for (i = 0; i < PTRS_PER_PTE; i++)
+ set_pte(&kasan_zero_pte[i],
+ pfn_pte(virt_to_pfn(kasan_zero_page), PAGE_KERNEL_RO));
+
memset(kasan_zero_page, 0, PAGE_SIZE);
cpu_replace_ttbr1(swapper_pg_dir);
This works, as far as I can tell. However, I still need the patch
below to make sure that the KAsan zero page is mapped read-only. (The
reason is that, depending on the alignment of the regions,
kasan_populate_zero_shadow() may never call
zero_[pud|pmd|pte]_populate())
Before this patch (and my change), the KAsan shadow regions looks like this:
0xffffff8000000000-0xffffff8200800000 8200M RW NX SHD AF UXN MEM/NORMAL
0xffffff8200800000-0xffffff8200c00000 4M RW NX SHD AF BLK UXN MEM/NORMAL
0xffffff8200c00000-0xffffff8800000000 24564M RW NX SHD AF UXN MEM/NORMAL
0xffffff8800000000-0xffffff8820200000 514M RW NX SHD AF BLK UXN MEM/NORMAL
and after:
0xffffff8000000000-0xffffff8200800000 8200M ro NX SHD AF UXN MEM/NORMAL
0xffffff8200800000-0xffffff8200c00000 4M RW NX SHD AF BLK UXN MEM/NORMAL
0xffffff8200c00000-0xffffff8800000000 24564M ro NX SHD AF UXN MEM/NORMAL
0xffffff8800000000-0xffffff8820200000 514M RW NX SHD AF BLK UXN MEM/NORMAL
directly, so we
+ * should make sure that it maps the zero page read-only.
+ */
+ for (i = 0; i < PTRS_PER_PTE; i++)
+ set_pte(&kasan_zero_pte[i],
+ pfn_pte(virt_to_pfn(kasan_zero_page), PAGE_KERNEL_RO));
+
memset(kasan_zero_page, 0, PAGE_SIZE);
cpu_replace_ttbr1(swapper_pg_dir);
On Fri, Jan 08, 2016 at 11:59:44AM +0100, Ard Biesheuvel wrote:
quoted hunk
This works, as far as I can tell. However, I still need the patch
below to make sure that the KAsan zero page is mapped read-only. (The
reason is that, depending on the alignment of the regions,
kasan_populate_zero_shadow() may never call
zero_[pud|pmd|pte]_populate())
Before this patch (and my change), the KAsan shadow regions looks like this:
0xffffff8000000000-0xffffff8200800000 8200M RW NX SHD AF UXN MEM/NORMAL
0xffffff8200800000-0xffffff8200c00000 4M RW NX SHD AF BLK UXN MEM/NORMAL
0xffffff8200c00000-0xffffff8800000000 24564M RW NX SHD AF UXN MEM/NORMAL
0xffffff8800000000-0xffffff8820200000 514M RW NX SHD AF BLK UXN MEM/NORMAL
and after:
0xffffff8000000000-0xffffff8200800000 8200M ro NX SHD AF UXN MEM/NORMAL
0xffffff8200800000-0xffffff8200c00000 4M RW NX SHD AF BLK UXN MEM/NORMAL
0xffffff8200c00000-0xffffff8800000000 24564M ro NX SHD AF UXN MEM/NORMAL
0xffffff8800000000-0xffffff8820200000 514M RW NX SHD AF BLK UXN MEM/NORMAL
---------8<--------------
directly, so we
+ * should make sure that it maps the zero page read-only.
+ */
+ for (i = 0; i < PTRS_PER_PTE; i++)
+ set_pte(&kasan_zero_pte[i],
+ pfn_pte(virt_to_pfn(kasan_zero_page), PAGE_KERNEL_RO));
+
memset(kasan_zero_page, 0, PAGE_SIZE);
cpu_replace_ttbr1(swapper_pg_dir);
That's complementary to my patch. Could you please send it with a proper
description so that Will can pick them up? For your patch:
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
--
Catalin