* [PATCH] arm64: mm: fix thinko in non-global page table attribute check
@ 2018-02-23 18:04 Ard Biesheuvel
2018-02-23 18:06 ` Ard Biesheuvel
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2018-02-23 18:04 UTC (permalink / raw)
To: linux-arm-kernel
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 related [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
` (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
end of thread, other threads:[~2018-02-26 14:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).