Several accesses of pgprot values are incorrect when compiled with
STRICT_MM_TYPECHECKS. Use the appropriate pgprot_val/__pgprot wrappers
to access the structures appropriately.
Signed-off-by: Laura Abbott <[email protected]>
---
Found while working on the set_memory_* work
---
arch/arm64/mm/mmu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index c2fa6b5..83a1162 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -146,7 +146,7 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
if (((addr | next | phys) & ~CONT_MASK) == 0) {
/* a block of CONT_PTES */
__populate_init_pte(pte, addr, next, phys,
- prot | __pgprot(PTE_CONT));
+ __pgprot(pgprot_val(prot) | PTE_CONT));
} else {
/*
* If the range being split is already inside of a
@@ -475,7 +475,7 @@ void mark_rodata_ro(void)
{
create_mapping_late(__pa(_stext), (unsigned long)_stext,
(unsigned long)_etext - (unsigned long)_stext,
- PAGE_KERNEL_EXEC | PTE_RDONLY);
+ __pgprot(pgprot_val(PAGE_KERNEL_EXEC) | PTE_RDONLY));
}
#endif
@@ -691,7 +691,7 @@ void __set_fixmap(enum fixed_addresses idx,
void *__init fixmap_remap_fdt(phys_addr_t dt_phys)
{
const u64 dt_virt_base = __fix_to_virt(FIX_FDT);
- pgprot_t prot = PAGE_KERNEL | PTE_RDONLY;
+ pgprot_t prot = __pgprot(pgprot_val(PAGE_KERNEL) | PTE_RDONLY);
int size, offset;
void *dt_virt;
--
2.5.0
PAGE_S2_DEVICE is a pgprot val and needs to be accessed using the proper
accessors. Switch to these accessors to avoid errors with
STRICT_MM_TYPECHECK.
Signed-off-by: Laura Abbott <[email protected]>
---
Found in the course of other work
---
arch/arm/kvm/mmu.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index 6984342..43f8162 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -213,7 +213,8 @@ static void unmap_ptes(struct kvm *kvm, pmd_t *pmd,
kvm_tlb_flush_vmid_ipa(kvm, addr);
/* No need to invalidate the cache for device mappings */
- if ((pte_val(old_pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE)
+ if ((pte_val(old_pte) & pgprot_val(PAGE_S2_DEVICE)) !=
+ pgprot_val(PAGE_S2_DEVICE))
kvm_flush_dcache_pte(old_pte);
put_page(virt_to_page(pte));
@@ -306,7 +307,8 @@ static void stage2_flush_ptes(struct kvm *kvm, pmd_t *pmd,
pte = pte_offset_kernel(pmd, addr);
do {
if (!pte_none(*pte) &&
- (pte_val(*pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE)
+ (pte_val(*pte) & pgprot_val(PAGE_S2_DEVICE)) !=
+ pgprot_val(PAGE_S2_DEVICE))
kvm_flush_dcache_pte(*pte);
} while (pte++, addr += PAGE_SIZE, addr != end);
}
--
2.5.0
Hi Laura,
On 11 November 2015 at 03:03, Laura Abbott <[email protected]> wrote:
>
> Several accesses of pgprot values are incorrect when compiled with
> STRICT_MM_TYPECHECKS. Use the appropriate pgprot_val/__pgprot wrappers
> to access the structures appropriately.
>
I spotted 2 out of these, and Catalin has already queued fixes for
them (see below)
> Signed-off-by: Laura Abbott <[email protected]>
> ---
> Found while working on the set_memory_* work
> ---
> arch/arm64/mm/mmu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index c2fa6b5..83a1162 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -146,7 +146,7 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
> if (((addr | next | phys) & ~CONT_MASK) == 0) {
> /* a block of CONT_PTES */
> __populate_init_pte(pte, addr, next, phys,
> - prot | __pgprot(PTE_CONT));
> + __pgprot(pgprot_val(prot) | PTE_CONT));
Got this one
> } else {
> /*
> * If the range being split is already inside of a
> @@ -475,7 +475,7 @@ void mark_rodata_ro(void)
> {
> create_mapping_late(__pa(_stext), (unsigned long)_stext,
> (unsigned long)_etext - (unsigned long)_stext,
> - PAGE_KERNEL_EXEC | PTE_RDONLY);
> + __pgprot(pgprot_val(PAGE_KERNEL_EXEC) | PTE_RDONLY));
>
This needs PAGE_KERNEL_RO (which was just introduced). The reason is
that PAGE_KERNEL_EXEC has PTE_WRITE set as well, making the range
writeable under the ARMv8.1 DBM feature, that manages the dirty bit in
hardware (writing to a page with the PTE_RDONLY and PTE_WRITE bits
both set will clear the PTE_RDONLY bit in that case)
> }
> #endif
> @@ -691,7 +691,7 @@ void __set_fixmap(enum fixed_addresses idx,
> void *__init fixmap_remap_fdt(phys_addr_t dt_phys)
> {
> const u64 dt_virt_base = __fix_to_virt(FIX_FDT);
> - pgprot_t prot = PAGE_KERNEL | PTE_RDONLY;
> + pgprot_t prot = __pgprot(pgprot_val(PAGE_KERNEL) | PTE_RDONLY);
Got this one as well (using PAGE_KERNEL_RO)
> int size, offset;
> void *dt_virt;
>
> --
> 2.5.0
>
On 11 November 2015 at 06:51, Ard Biesheuvel <[email protected]> wrote:
> Hi Laura,
>
> On 11 November 2015 at 03:03, Laura Abbott <[email protected]> wrote:
>>
>> Several accesses of pgprot values are incorrect when compiled with
>> STRICT_MM_TYPECHECKS. Use the appropriate pgprot_val/__pgprot wrappers
>> to access the structures appropriately.
>>
>
> I spotted 2 out of these, and Catalin has already queued fixes for
> them (see below)
>
>> Signed-off-by: Laura Abbott <[email protected]>
>> ---
>> Found while working on the set_memory_* work
>> ---
>> arch/arm64/mm/mmu.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index c2fa6b5..83a1162 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -146,7 +146,7 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
>> if (((addr | next | phys) & ~CONT_MASK) == 0) {
>> /* a block of CONT_PTES */
>> __populate_init_pte(pte, addr, next, phys,
>> - prot | __pgprot(PTE_CONT));
>> + __pgprot(pgprot_val(prot) | PTE_CONT));
>
> Got this one
>
>> } else {
>> /*
>> * If the range being split is already inside of a
>> @@ -475,7 +475,7 @@ void mark_rodata_ro(void)
>> {
>> create_mapping_late(__pa(_stext), (unsigned long)_stext,
>> (unsigned long)_etext - (unsigned long)_stext,
>> - PAGE_KERNEL_EXEC | PTE_RDONLY);
>> + __pgprot(pgprot_val(PAGE_KERNEL_EXEC) | PTE_RDONLY));
>>
>
> This needs PAGE_KERNEL_RO (which was just introduced). The reason is
> that PAGE_KERNEL_EXEC has PTE_WRITE set as well, making the range
> writeable under the ARMv8.1 DBM feature, that manages the dirty bit in
> hardware (writing to a page with the PTE_RDONLY and PTE_WRITE bits
> both set will clear the PTE_RDONLY bit in that case)
>
...only you'd obviously need to clear the PTE_PXN bit (or introduce a
new PAGE_KERNEL_xx define?)
>> }
>> #endif
>> @@ -691,7 +691,7 @@ void __set_fixmap(enum fixed_addresses idx,
>> void *__init fixmap_remap_fdt(phys_addr_t dt_phys)
>> {
>> const u64 dt_virt_base = __fix_to_virt(FIX_FDT);
>> - pgprot_t prot = PAGE_KERNEL | PTE_RDONLY;
>> + pgprot_t prot = __pgprot(pgprot_val(PAGE_KERNEL) | PTE_RDONLY);
>
> Got this one as well (using PAGE_KERNEL_RO)
>
>> int size, offset;
>> void *dt_virt;
>>
>> --
>> 2.5.0
>>
The permissions in mark_rodata_ro trigger a build error
with STRICT_MM_TYPECHECKS. Fix this by introducing
PAGE_KERNEL_ROX for the same reasons as PAGE_KERNEL_RO.
>From Ard:
"PAGE_KERNEL_EXEC has PTE_WRITE set as well, making the range
writeable under the ARMv8.1 DBM feature, that manages the
dirty bit in hardware (writing to a page with the PTE_RDONLY
and PTE_WRITE bits both set will clear the PTE_RDONLY bit in that case)"
Signed-off-by: Laura Abbott <[email protected]>
---
arch/arm64/include/asm/pgtable.h | 1 +
arch/arm64/mm/mmu.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 9819a94..7e074f9 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -81,6 +81,7 @@ extern void __pgd_error(const char *file, int line, unsigned long val);
#define PAGE_KERNEL __pgprot(_PAGE_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY | PTE_WRITE)
#define PAGE_KERNEL_RO __pgprot(_PAGE_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY | PTE_RDONLY)
+#define PAGE_KERNEL_ROX __pgprot(_PAGE_DEFAULT | PTE_UXN | PTE_DIRTY | PTE_RDONLY)
#define PAGE_KERNEL_EXEC __pgprot(_PAGE_DEFAULT | PTE_UXN | PTE_DIRTY | PTE_WRITE)
#define PAGE_KERNEL_EXEC_CONT __pgprot(_PAGE_DEFAULT | PTE_UXN | PTE_DIRTY | PTE_WRITE | PTE_CONT)
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 9ca5996..6598c48 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -475,7 +475,7 @@ void mark_rodata_ro(void)
{
create_mapping_late(__pa(_stext), (unsigned long)_stext,
(unsigned long)_etext - (unsigned long)_stext,
- PAGE_KERNEL_EXEC | PTE_RDONLY);
+ PAGE_KERNEL_ROX);
}
#endif
--
2.5.0
On 11 November 2015 at 03:03, Laura Abbott <[email protected]> wrote:
>
> PAGE_S2_DEVICE is a pgprot val and needs to be accessed using the proper
> accessors. Switch to these accessors to avoid errors with
> STRICT_MM_TYPECHECK.
>
> Signed-off-by: Laura Abbott <[email protected]>
> ---
> Found in the course of other work
Already fixed here:
http://thread.gmane.org/gmane.comp.emulators.kvm.devel/142953
Looks like we may need a mutex :-)
> ---
> arch/arm/kvm/mmu.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index 6984342..43f8162 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -213,7 +213,8 @@ static void unmap_ptes(struct kvm *kvm, pmd_t *pmd,
> kvm_tlb_flush_vmid_ipa(kvm, addr);
>
> /* No need to invalidate the cache for device mappings */
> - if ((pte_val(old_pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE)
> + if ((pte_val(old_pte) & pgprot_val(PAGE_S2_DEVICE)) !=
> + pgprot_val(PAGE_S2_DEVICE))
> kvm_flush_dcache_pte(old_pte);
>
> put_page(virt_to_page(pte));
> @@ -306,7 +307,8 @@ static void stage2_flush_ptes(struct kvm *kvm, pmd_t *pmd,
> pte = pte_offset_kernel(pmd, addr);
> do {
> if (!pte_none(*pte) &&
> - (pte_val(*pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE)
> + (pte_val(*pte) & pgprot_val(PAGE_S2_DEVICE)) !=
> + pgprot_val(PAGE_S2_DEVICE))
> kvm_flush_dcache_pte(*pte);
> } while (pte++, addr += PAGE_SIZE, addr != end);
> }
> --
> 2.5.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 12 November 2015 at 21:21, Laura Abbott <[email protected]> wrote:
> The permissions in mark_rodata_ro trigger a build error
> with STRICT_MM_TYPECHECKS. Fix this by introducing
> PAGE_KERNEL_ROX for the same reasons as PAGE_KERNEL_RO.
Forgive the bikeshedding but perhaps PAGE_KERNEL_EXEC_RO is more consistent?
> From Ard:
>
> "PAGE_KERNEL_EXEC has PTE_WRITE set as well, making the range
> writeable under the ARMv8.1 DBM feature, that manages the
> dirty bit in hardware (writing to a page with the PTE_RDONLY
> and PTE_WRITE bits both set will clear the PTE_RDONLY bit in that case)"
>
> Signed-off-by: Laura Abbott <[email protected]>
Acked-by: Ard Biesheuvel <[email protected]>
> ---
> arch/arm64/include/asm/pgtable.h | 1 +
> arch/arm64/mm/mmu.c | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 9819a94..7e074f9 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -81,6 +81,7 @@ extern void __pgd_error(const char *file, int line, unsigned long val);
>
> #define PAGE_KERNEL __pgprot(_PAGE_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY | PTE_WRITE)
> #define PAGE_KERNEL_RO __pgprot(_PAGE_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY | PTE_RDONLY)
> +#define PAGE_KERNEL_ROX __pgprot(_PAGE_DEFAULT | PTE_UXN | PTE_DIRTY | PTE_RDONLY)
> #define PAGE_KERNEL_EXEC __pgprot(_PAGE_DEFAULT | PTE_UXN | PTE_DIRTY | PTE_WRITE)
> #define PAGE_KERNEL_EXEC_CONT __pgprot(_PAGE_DEFAULT | PTE_UXN | PTE_DIRTY | PTE_WRITE | PTE_CONT)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 9ca5996..6598c48 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -475,7 +475,7 @@ void mark_rodata_ro(void)
> {
> create_mapping_late(__pa(_stext), (unsigned long)_stext,
> (unsigned long)_etext - (unsigned long)_stext,
> - PAGE_KERNEL_EXEC | PTE_RDONLY);
> + PAGE_KERNEL_ROX);
>
> }
> #endif
> --
> 2.5.0
>
On 11/14/2015 11:38 PM, Ard Biesheuvel wrote:
> On 12 November 2015 at 21:21, Laura Abbott <[email protected]> wrote:
>> The permissions in mark_rodata_ro trigger a build error
>> with STRICT_MM_TYPECHECKS. Fix this by introducing
>> PAGE_KERNEL_ROX for the same reasons as PAGE_KERNEL_RO.
>
> Forgive the bikeshedding but perhaps PAGE_KERNEL_EXEC_RO is more consistent?
>
powerpc was using PAGE_KERNEL_ROX so I went with that. Blame powerpc ;)
>> From Ard:
>>
>> "PAGE_KERNEL_EXEC has PTE_WRITE set as well, making the range
>> writeable under the ARMv8.1 DBM feature, that manages the
>> dirty bit in hardware (writing to a page with the PTE_RDONLY
>> and PTE_WRITE bits both set will clear the PTE_RDONLY bit in that case)"
>>
>> Signed-off-by: Laura Abbott <[email protected]>
>
> Acked-by: Ard Biesheuvel <[email protected]>
>
Thanks,
Laura
On 16 November 2015 at 23:50, Laura Abbott <[email protected]> wrote:
> On 11/14/2015 11:38 PM, Ard Biesheuvel wrote:
>>
>> On 12 November 2015 at 21:21, Laura Abbott <[email protected]>
>> wrote:
>>>
>>> The permissions in mark_rodata_ro trigger a build error
>>> with STRICT_MM_TYPECHECKS. Fix this by introducing
>>> PAGE_KERNEL_ROX for the same reasons as PAGE_KERNEL_RO.
>>
>>
>> Forgive the bikeshedding but perhaps PAGE_KERNEL_EXEC_RO is more
>> consistent?
>>
>
> powerpc was using PAGE_KERNEL_ROX so I went with that. Blame powerpc ;)
>
OK, in that case, let's keep PAGE_KERNEL_ROX
>>>
>>> From Ard:
>>>
>>> "PAGE_KERNEL_EXEC has PTE_WRITE set as well, making the range
>>> writeable under the ARMv8.1 DBM feature, that manages the
>>> dirty bit in hardware (writing to a page with the PTE_RDONLY
>>> and PTE_WRITE bits both set will clear the PTE_RDONLY bit in that case)"
>>>
>>> Signed-off-by: Laura Abbott <[email protected]>
>>
>>
>> Acked-by: Ard Biesheuvel <[email protected]>
>>
>
> Thanks,
> Laura
>
On Thu, Nov 12, 2015 at 12:21:10PM -0800, Laura Abbott wrote:
> The permissions in mark_rodata_ro trigger a build error
> with STRICT_MM_TYPECHECKS. Fix this by introducing
> PAGE_KERNEL_ROX for the same reasons as PAGE_KERNEL_RO.
> From Ard:
>
> "PAGE_KERNEL_EXEC has PTE_WRITE set as well, making the range
> writeable under the ARMv8.1 DBM feature, that manages the
> dirty bit in hardware (writing to a page with the PTE_RDONLY
> and PTE_WRITE bits both set will clear the PTE_RDONLY bit in that case)"
>
> Signed-off-by: Laura Abbott <[email protected]>
Applied. Thanks.
--
Catalin