* [PATCH 1/2] arm64: Do not pass tagged addresses to __is_lm_address()
2021-02-01 19:06 [PATCH 0/2] arm64: Further fixes to the linear address checking Catalin Marinas
@ 2021-02-01 19:06 ` Catalin Marinas
2021-02-01 19:06 ` [PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros Catalin Marinas
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2021-02-01 19:06 UTC (permalink / raw)
To: Ard Biesheuvel, Linus Torvalds
Cc: linux-arm-kernel, Vincenzo Frascino, Will Deacon, Mark Rutland, stable
Commit 519ea6f1c82f ("arm64: Fix kernel address detection of
__is_lm_address()") fixed the incorrect validation of addresses below
PAGE_OFFSET. However, it no longer allowed tagged addresses to be passed
to virt_addr_valid().
Fix this by explicitly resetting the pointer tag prior to invoking
__is_lm_address(). This is consistent with the __lm_to_phys() macro.
Fixes: 519ea6f1c82f ("arm64: Fix kernel address detection of __is_lm_address()")
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: <stable@vger.kernel.org> # 5.4.x
Cc: Will Deacon <will@kernel.org>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
---
arch/arm64/include/asm/memory.h | 2 +-
arch/arm64/mm/physaddr.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 99d7e1494aaa..3c1aaa522cbd 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -332,7 +332,7 @@ static inline void *phys_to_virt(phys_addr_t x)
#endif /* !CONFIG_SPARSEMEM_VMEMMAP || CONFIG_DEBUG_VIRTUAL */
#define virt_addr_valid(addr) ({ \
- __typeof__(addr) __addr = addr; \
+ __typeof__(addr) __addr = __tag_reset(addr); \
__is_lm_address(__addr) && pfn_valid(virt_to_pfn(__addr)); \
})
diff --git a/arch/arm64/mm/physaddr.c b/arch/arm64/mm/physaddr.c
index 67a9ba9eaa96..cde44c13dda1 100644
--- a/arch/arm64/mm/physaddr.c
+++ b/arch/arm64/mm/physaddr.c
@@ -9,7 +9,7 @@
phys_addr_t __virt_to_phys(unsigned long x)
{
- WARN(!__is_lm_address(x),
+ WARN(!__is_lm_address(__tag_reset(x)),
"virt_to_phys used for non-linear address: %pK (%pS)\n",
(void *)x,
(void *)x);
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 1/2] arm64: Do not pass tagged addresses to __is_lm_address()
@ 2021-02-01 19:06 ` Catalin Marinas
0 siblings, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2021-02-01 19:06 UTC (permalink / raw)
To: Ard Biesheuvel, Linus Torvalds
Cc: Mark Rutland, Vincenzo Frascino, Will Deacon, stable, linux-arm-kernel
Commit 519ea6f1c82f ("arm64: Fix kernel address detection of
__is_lm_address()") fixed the incorrect validation of addresses below
PAGE_OFFSET. However, it no longer allowed tagged addresses to be passed
to virt_addr_valid().
Fix this by explicitly resetting the pointer tag prior to invoking
__is_lm_address(). This is consistent with the __lm_to_phys() macro.
Fixes: 519ea6f1c82f ("arm64: Fix kernel address detection of __is_lm_address()")
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: <stable@vger.kernel.org> # 5.4.x
Cc: Will Deacon <will@kernel.org>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
---
arch/arm64/include/asm/memory.h | 2 +-
arch/arm64/mm/physaddr.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 99d7e1494aaa..3c1aaa522cbd 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -332,7 +332,7 @@ static inline void *phys_to_virt(phys_addr_t x)
#endif /* !CONFIG_SPARSEMEM_VMEMMAP || CONFIG_DEBUG_VIRTUAL */
#define virt_addr_valid(addr) ({ \
- __typeof__(addr) __addr = addr; \
+ __typeof__(addr) __addr = __tag_reset(addr); \
__is_lm_address(__addr) && pfn_valid(virt_to_pfn(__addr)); \
})
diff --git a/arch/arm64/mm/physaddr.c b/arch/arm64/mm/physaddr.c
index 67a9ba9eaa96..cde44c13dda1 100644
--- a/arch/arm64/mm/physaddr.c
+++ b/arch/arm64/mm/physaddr.c
@@ -9,7 +9,7 @@
phys_addr_t __virt_to_phys(unsigned long x)
{
- WARN(!__is_lm_address(x),
+ WARN(!__is_lm_address(__tag_reset(x)),
"virt_to_phys used for non-linear address: %pK (%pS)\n",
(void *)x,
(void *)x);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros
2021-02-01 19:06 [PATCH 0/2] arm64: Further fixes to the linear address checking Catalin Marinas
2021-02-01 19:06 ` Catalin Marinas
@ 2021-02-01 19:06 ` Catalin Marinas
2021-02-01 23:05 ` Ard Biesheuvel
2021-02-01 19:33 ` [PATCH 0/2] arm64: Further fixes to the linear address checking Linus Torvalds
2021-02-02 17:46 ` Catalin Marinas
3 siblings, 1 reply; 10+ messages in thread
From: Catalin Marinas @ 2021-02-01 19:06 UTC (permalink / raw)
To: Ard Biesheuvel, Linus Torvalds
Cc: Mark Rutland, Vincenzo Frascino, Will Deacon, linux-arm-kernel
Because of the tagged addresses, the __is_lm_address() and
__lm_to_phys() macros grew to some harder to understand bitwise
operations using PAGE_OFFSET. Since these macros only accept untagged
addresses, use a simple subtract operation.
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
---
arch/arm64/include/asm/memory.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 3c1aaa522cbd..ff4732785c32 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -251,9 +251,9 @@ static inline const void *__tag_set(const void *addr, u8 tag)
* lives in the [PAGE_OFFSET, PAGE_END) interval at the bottom of the
* kernel's TTBR1 address range.
*/
-#define __is_lm_address(addr) (((u64)(addr) ^ PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET))
+#define __is_lm_address(addr) (((u64)(addr) - PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET))
-#define __lm_to_phys(addr) (((addr) & ~PAGE_OFFSET) + PHYS_OFFSET)
+#define __lm_to_phys(addr) (((addr) - PAGE_OFFSET) + PHYS_OFFSET)
#define __kimg_to_phys(addr) ((addr) - kimage_voffset)
#define __virt_to_phys_nodebug(x) ({ \
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros
2021-02-01 19:06 ` [PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros Catalin Marinas
@ 2021-02-01 23:05 ` Ard Biesheuvel
2021-02-01 23:17 ` Linus Torvalds
2021-02-02 11:36 ` Catalin Marinas
0 siblings, 2 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2021-02-01 23:05 UTC (permalink / raw)
To: Catalin Marinas
Cc: Mark Rutland, Will Deacon, Vincenzo Frascino, Linus Torvalds, Linux ARM
On Mon, 1 Feb 2021 at 20:06, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> Because of the tagged addresses, the __is_lm_address() and
> __lm_to_phys() macros grew to some harder to understand bitwise
> operations using PAGE_OFFSET. Since these macros only accept untagged
> addresses, use a simple subtract operation.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
> arch/arm64/include/asm/memory.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 3c1aaa522cbd..ff4732785c32 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -251,9 +251,9 @@ static inline const void *__tag_set(const void *addr, u8 tag)
> * lives in the [PAGE_OFFSET, PAGE_END) interval at the bottom of the
> * kernel's TTBR1 address range.
> */
> -#define __is_lm_address(addr) (((u64)(addr) ^ PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET))
> +#define __is_lm_address(addr) (((u64)(addr) - PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET))
>
Why still subtract PAGE_OFFSET on both sides of the comparison here?
> -#define __lm_to_phys(addr) (((addr) & ~PAGE_OFFSET) + PHYS_OFFSET)
> +#define __lm_to_phys(addr) (((addr) - PAGE_OFFSET) + PHYS_OFFSET)
> #define __kimg_to_phys(addr) ((addr) - kimage_voffset)
>
> #define __virt_to_phys_nodebug(x) ({ \
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros
2021-02-01 23:05 ` Ard Biesheuvel
@ 2021-02-01 23:17 ` Linus Torvalds
2021-02-02 11:36 ` Catalin Marinas
1 sibling, 0 replies; 10+ messages in thread
From: Linus Torvalds @ 2021-02-01 23:17 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Mark Rutland, Catalin Marinas, Vincenzo Frascino, Will Deacon, Linux ARM
On Mon, Feb 1, 2021 at 3:05 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Why still subtract PAGE_OFFSET on both sides of the comparison here?
Because otherwise you'd have to use 'addr' twice in a macro, and write
it something like
#define __is_lm_address(addr) \
(((u64)(addr) >= PAGE_OFFSET) && < ((u64)(addr) < PAGE_END))
which then would require you to do a local variable in a statement
expression to avoid the double evaluation..
Linus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros
2021-02-01 23:05 ` Ard Biesheuvel
2021-02-01 23:17 ` Linus Torvalds
@ 2021-02-02 11:36 ` Catalin Marinas
2021-02-02 11:40 ` Ard Biesheuvel
1 sibling, 1 reply; 10+ messages in thread
From: Catalin Marinas @ 2021-02-02 11:36 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Mark Rutland, Will Deacon, Vincenzo Frascino, Linus Torvalds, Linux ARM
On Tue, Feb 02, 2021 at 12:05:10AM +0100, Ard Biesheuvel wrote:
> On Mon, 1 Feb 2021 at 20:06, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > Because of the tagged addresses, the __is_lm_address() and
> > __lm_to_phys() macros grew to some harder to understand bitwise
> > operations using PAGE_OFFSET. Since these macros only accept untagged
> > addresses, use a simple subtract operation.
> >
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > ---
> > arch/arm64/include/asm/memory.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> > index 3c1aaa522cbd..ff4732785c32 100644
> > --- a/arch/arm64/include/asm/memory.h
> > +++ b/arch/arm64/include/asm/memory.h
> > @@ -251,9 +251,9 @@ static inline const void *__tag_set(const void *addr, u8 tag)
> > * lives in the [PAGE_OFFSET, PAGE_END) interval at the bottom of the
> > * kernel's TTBR1 address range.
> > */
> > -#define __is_lm_address(addr) (((u64)(addr) ^ PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET))
> > +#define __is_lm_address(addr) (((u64)(addr) - PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET))
>
> Why still subtract PAGE_OFFSET on both sides of the comparison here?
Not sure what you mean (Linus replied on the local variable aspect).
With signed arithmetics, the above would be equivalent to addr < PAGE_END
but it wouldn't cover the addr < PAGE_OFFSET case. So we rely on the
unsigned wrap-around to detect this.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros
2021-02-02 11:36 ` Catalin Marinas
@ 2021-02-02 11:40 ` Ard Biesheuvel
0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2021-02-02 11:40 UTC (permalink / raw)
To: Catalin Marinas
Cc: Mark Rutland, Will Deacon, Vincenzo Frascino, Linus Torvalds, Linux ARM
On Tue, 2 Feb 2021 at 12:36, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Tue, Feb 02, 2021 at 12:05:10AM +0100, Ard Biesheuvel wrote:
> > On Mon, 1 Feb 2021 at 20:06, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > Because of the tagged addresses, the __is_lm_address() and
> > > __lm_to_phys() macros grew to some harder to understand bitwise
> > > operations using PAGE_OFFSET. Since these macros only accept untagged
> > > addresses, use a simple subtract operation.
> > >
> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Will Deacon <will@kernel.org>
> > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > ---
> > > arch/arm64/include/asm/memory.h | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> > > index 3c1aaa522cbd..ff4732785c32 100644
> > > --- a/arch/arm64/include/asm/memory.h
> > > +++ b/arch/arm64/include/asm/memory.h
> > > @@ -251,9 +251,9 @@ static inline const void *__tag_set(const void *addr, u8 tag)
> > > * lives in the [PAGE_OFFSET, PAGE_END) interval at the bottom of the
> > > * kernel's TTBR1 address range.
> > > */
> > > -#define __is_lm_address(addr) (((u64)(addr) ^ PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET))
> > > +#define __is_lm_address(addr) (((u64)(addr) - PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET))
> >
> > Why still subtract PAGE_OFFSET on both sides of the comparison here?
>
> Not sure what you mean (Linus replied on the local variable aspect).
>
> With signed arithmetics, the above would be equivalent to addr < PAGE_END
> but it wouldn't cover the addr < PAGE_OFFSET case. So we rely on the
> unsigned wrap-around to detect this.
>
Never mind, I just got confused. These changes look fine.
For the series,
Acled-by: Ard Biesheuvel <ardb@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] arm64: Further fixes to the linear address checking
2021-02-01 19:06 [PATCH 0/2] arm64: Further fixes to the linear address checking Catalin Marinas
2021-02-01 19:06 ` Catalin Marinas
2021-02-01 19:06 ` [PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros Catalin Marinas
@ 2021-02-01 19:33 ` Linus Torvalds
2021-02-02 17:46 ` Catalin Marinas
3 siblings, 0 replies; 10+ messages in thread
From: Linus Torvalds @ 2021-02-01 19:33 UTC (permalink / raw)
To: Catalin Marinas
Cc: Mark Rutland, Vincenzo Frascino, Will Deacon, Ard Biesheuvel, Linux ARM
On Mon, Feb 1, 2021 at 11:06 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> The first patch attempts to fix virt_addr_valid() macro for tagged
> addresses. The second patch replaces the bitwise operations with a
> subtract. The resulting image is slightly smaller.
Thanks, this looks very sane to me.
Linus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/2] arm64: Further fixes to the linear address checking
2021-02-01 19:06 [PATCH 0/2] arm64: Further fixes to the linear address checking Catalin Marinas
` (2 preceding siblings ...)
2021-02-01 19:33 ` [PATCH 0/2] arm64: Further fixes to the linear address checking Linus Torvalds
@ 2021-02-02 17:46 ` Catalin Marinas
3 siblings, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2021-02-02 17:46 UTC (permalink / raw)
To: Ard Biesheuvel, Linus Torvalds, Catalin Marinas
Cc: Mark Rutland, Vincenzo Frascino, Will Deacon, linux-arm-kernel
On Mon, 1 Feb 2021 19:06:32 +0000, Catalin Marinas wrote:
> There some history to virt_addr_valid() that goes back to 5.4. It seems
> to have been ok prior to this kernel, however, the VA reshuffling in
> commit 14c127c957c1 ("arm64: mm: Flip kernel VA space") broke it. The
> subsequent fix 68dd8ef32162 ("arm64: memory: Fix virt_addr_valid() using
> __is_lm_address()") partially addressed it but it left virt_addr_valid()
> returning true for user addresses (or NULL).
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[1/2] arm64: Do not pass tagged addresses to __is_lm_address()
https://git.kernel.org/arm64/c/91cb2c8b072e
[2/2] arm64: Use simpler arithmetics for the linear map macros
https://git.kernel.org/arm64/c/22cd5edb2d9c
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread