* [PATCH] arm64: mm: fix thinko in non-global page table attribute check
2018-02-23 18:04 [PATCH] arm64: mm: fix thinko in non-global page table attribute check Ard Biesheuvel
@ 2018-02-23 18:06 ` Ard Biesheuvel
2018-02-23 18:16 ` Mark Rutland
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2018-02-23 18:06 UTC (permalink / raw)
To: linux-arm-kernel
(corrected Nicolas's email address - beware when replying)
On 23 February 2018 at 18:04, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> The routine pgattr_change_is_safe() was extended in commit 4e6020565596
> ("arm64: mm: Permit transitioning from Global to Non-Global without BBM")
> to permit changing the nG attribute from not set to set, but did so in a
> way that inadvertently disallows such changes if other permitted attribute
> changes take place at the same time. So update the code to take this into
> account.
>
> Fixes: 4e6020565596 ("arm64: mm: Permit transitioning from Global to ...")
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/mm/mmu.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 451f96f3377c..5bdc2c4db9ad 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -107,7 +107,7 @@ static bool pgattr_change_is_safe(u64 old, u64 new)
> * The following mapping attributes may be updated in live
> * kernel mappings without the need for break-before-make.
> */
> - static const pteval_t mask = PTE_PXN | PTE_RDONLY | PTE_WRITE;
> + static const pteval_t mask = PTE_PXN | PTE_RDONLY | PTE_WRITE | PTE_NG;
>
> /* creating or taking down mappings is always safe */
> if (old == 0 || new == 0)
> @@ -117,9 +117,9 @@ static bool pgattr_change_is_safe(u64 old, u64 new)
> if ((old | new) & PTE_CONT)
> return false;
>
> - /* Transitioning from Global to Non-Global is safe */
> - if (((old ^ new) == PTE_NG) && (new & PTE_NG))
> - return true;
> + /* Transitioning from Non-Global to Global is unsafe */
> + if (old & ~new & PTE_NG)
> + return false;
>
> return ((old ^ new) & ~mask) == 0;
> }
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] arm64: mm: fix thinko in non-global page table attribute check
2018-02-23 18:04 [PATCH] arm64: mm: fix thinko in non-global page table attribute check Ard Biesheuvel
2018-02-23 18:06 ` Ard Biesheuvel
@ 2018-02-23 18:16 ` Mark Rutland
2018-02-23 18:18 ` Marc Zyngier
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Mark Rutland @ 2018-02-23 18:16 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Feb 23, 2018 at 06:04:48PM +0000, Ard Biesheuvel wrote:
> The routine pgattr_change_is_safe() was extended in commit 4e6020565596
> ("arm64: mm: Permit transitioning from Global to Non-Global without BBM")
> to permit changing the nG attribute from not set to set, but did so in a
> way that inadvertently disallows such changes if other permitted attribute
> changes take place at the same time. So update the code to take this into
> account.
>
> Fixes: 4e6020565596 ("arm64: mm: Permit transitioning from Global to ...")
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
FWIW:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> arch/arm64/mm/mmu.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 451f96f3377c..5bdc2c4db9ad 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -107,7 +107,7 @@ static bool pgattr_change_is_safe(u64 old, u64 new)
> * The following mapping attributes may be updated in live
> * kernel mappings without the need for break-before-make.
> */
> - static const pteval_t mask = PTE_PXN | PTE_RDONLY | PTE_WRITE;
> + static const pteval_t mask = PTE_PXN | PTE_RDONLY | PTE_WRITE | PTE_NG;
>
> /* creating or taking down mappings is always safe */
> if (old == 0 || new == 0)
> @@ -117,9 +117,9 @@ static bool pgattr_change_is_safe(u64 old, u64 new)
> if ((old | new) & PTE_CONT)
> return false;
>
> - /* Transitioning from Global to Non-Global is safe */
> - if (((old ^ new) == PTE_NG) && (new & PTE_NG))
> - return true;
> + /* Transitioning from Non-Global to Global is unsafe */
> + if (old & ~new & PTE_NG)
> + return false;
>
> return ((old ^ new) & ~mask) == 0;
> }
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] arm64: mm: fix thinko in non-global page table attribute check
2018-02-23 18:04 [PATCH] arm64: mm: fix thinko in non-global page table attribute check Ard Biesheuvel
2018-02-23 18:06 ` Ard Biesheuvel
2018-02-23 18:16 ` Mark Rutland
@ 2018-02-23 18:18 ` Marc Zyngier
2018-02-26 9:51 ` Will Deacon
2018-02-26 14:27 ` Catalin Marinas
4 siblings, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2018-02-23 18:18 UTC (permalink / raw)
To: linux-arm-kernel
On 23/02/18 18:04, Ard Biesheuvel wrote:
> The routine pgattr_change_is_safe() was extended in commit 4e6020565596
> ("arm64: mm: Permit transitioning from Global to Non-Global without BBM")
> to permit changing the nG attribute from not set to set, but did so in a
> way that inadvertently disallows such changes if other permitted attribute
> changes take place at the same time. So update the code to take this into
> account.
>
> Fixes: 4e6020565596 ("arm64: mm: Permit transitioning from Global to ...")
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/mm/mmu.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 451f96f3377c..5bdc2c4db9ad 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -107,7 +107,7 @@ static bool pgattr_change_is_safe(u64 old, u64 new)
> * The following mapping attributes may be updated in live
> * kernel mappings without the need for break-before-make.
> */
> - static const pteval_t mask = PTE_PXN | PTE_RDONLY | PTE_WRITE;
> + static const pteval_t mask = PTE_PXN | PTE_RDONLY | PTE_WRITE | PTE_NG;
>
> /* creating or taking down mappings is always safe */
> if (old == 0 || new == 0)
> @@ -117,9 +117,9 @@ static bool pgattr_change_is_safe(u64 old, u64 new)
> if ((old | new) & PTE_CONT)
> return false;
>
> - /* Transitioning from Global to Non-Global is safe */
> - if (((old ^ new) == PTE_NG) && (new & PTE_NG))
> - return true;
> + /* Transitioning from Non-Global to Global is unsafe */
> + if (old & ~new & PTE_NG)
> + return false;
>
> return ((old ^ new) & ~mask) == 0;
> }
>
Nice one.
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] arm64: mm: fix thinko in non-global page table attribute check
2018-02-23 18:04 [PATCH] arm64: mm: fix thinko in non-global page table attribute check Ard Biesheuvel
` (2 preceding siblings ...)
2018-02-23 18:18 ` Marc Zyngier
@ 2018-02-26 9:51 ` Will Deacon
2018-02-26 14:27 ` Catalin Marinas
4 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2018-02-26 9:51 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Feb 23, 2018 at 06:04:48PM +0000, Ard Biesheuvel wrote:
> The routine pgattr_change_is_safe() was extended in commit 4e6020565596
> ("arm64: mm: Permit transitioning from Global to Non-Global without BBM")
> to permit changing the nG attribute from not set to set, but did so in a
> way that inadvertently disallows such changes if other permitted attribute
> changes take place at the same time. So update the code to take this into
> account.
>
> Fixes: 4e6020565596 ("arm64: mm: Permit transitioning from Global to ...")
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/mm/mmu.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 451f96f3377c..5bdc2c4db9ad 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -107,7 +107,7 @@ static bool pgattr_change_is_safe(u64 old, u64 new)
> * The following mapping attributes may be updated in live
> * kernel mappings without the need for break-before-make.
> */
> - static const pteval_t mask = PTE_PXN | PTE_RDONLY | PTE_WRITE;
> + static const pteval_t mask = PTE_PXN | PTE_RDONLY | PTE_WRITE | PTE_NG;
>
> /* creating or taking down mappings is always safe */
> if (old == 0 || new == 0)
> @@ -117,9 +117,9 @@ static bool pgattr_change_is_safe(u64 old, u64 new)
> if ((old | new) & PTE_CONT)
> return false;
>
> - /* Transitioning from Global to Non-Global is safe */
> - if (((old ^ new) == PTE_NG) && (new & PTE_NG))
> - return true;
> + /* Transitioning from Non-Global to Global is unsafe */
> + if (old & ~new & PTE_NG)
> + return false;
Oops, sorry about this:
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] arm64: mm: fix thinko in non-global page table attribute check
2018-02-23 18:04 [PATCH] arm64: mm: fix thinko in non-global page table attribute check Ard Biesheuvel
` (3 preceding siblings ...)
2018-02-26 9:51 ` Will Deacon
@ 2018-02-26 14:27 ` Catalin Marinas
4 siblings, 0 replies; 6+ messages in thread
From: Catalin Marinas @ 2018-02-26 14:27 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Feb 23, 2018 at 06:04:48PM +0000, Ard Biesheuvel wrote:
> The routine pgattr_change_is_safe() was extended in commit 4e6020565596
> ("arm64: mm: Permit transitioning from Global to Non-Global without BBM")
> to permit changing the nG attribute from not set to set, but did so in a
> way that inadvertently disallows such changes if other permitted attribute
> changes take place at the same time. So update the code to take this into
> account.
>
> Fixes: 4e6020565596 ("arm64: mm: Permit transitioning from Global to ...")
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Queued for 4.16. Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 6+ messages in thread