[PATCH 0/2] arm64: Further fixes to the linear address checking

[PATCH 0/2] arm64: Further fixes to the linear address checking

[Catalin Marinas]

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).

The recent commit 519ea6f1c82f ("arm64: Fix kernel address detection of
__is_lm_address()") fixed the NULL checking but broke the
virt_addr_valid() macro for tagged addresses.

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.

Catalin Marinas (2):
  arm64: Do not pass tagged addresses to __is_lm_address()
  arm64: Use simpler arithmetics for the linear map macros

 arch/arm64/include/asm/memory.h | 6 +++---
 arch/arm64/mm/physaddr.c        | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] arm64: Do not pass tagged addresses to __is_lm_address()

[Catalin Marinas]

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);

[PATCH 1/2] arm64: Do not pass tagged addresses to __is_lm_address()

[Catalin Marinas]

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

[PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros

[Catalin Marinas]

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

Re: [PATCH 0/2] arm64: Further fixes to the linear address checking

[Linus Torvalds]

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

Re: [PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros

[Ard Biesheuvel]

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

Re: [PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros

[Linus Torvalds]

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

Re: [PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros

[Catalin Marinas]

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

Re: [PATCH 2/2] arm64: Use simpler arithmetics for the linear map macros

[Ard Biesheuvel]

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

Re: [PATCH 0/2] arm64: Further fixes to the linear address checking

[Catalin Marinas]

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