[PATCH v2] arm64: ptr auth: Use get_random_u64 instead of _bytes

[PATCH v2] arm64: ptr auth: Use get_random_u64 instead of _bytes

[George Spelvin]

get_random_bytes() is approximately 4x the cost of two
get_random_u64() calls, because the former implements
anti-backtracking.

Because these are authentication keys, useless to an attacker
as soon as the kernel stops using them, there is no security
benefit from anti-backtracking.

Signed-off-by: George Spelvin <lkml@sdf.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
---
v2: Took out all the clever bitmap-based stuff and made a simple
    boring helper function to replace get_random_bytes(&key, 16).

 arch/arm64/include/asm/pointer_auth.h | 16 +++++++++++-----
 arch/arm64/kernel/pointer_auth.c      | 10 +++++-----
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
index 7a24bad1a58b8..1a4590d05785e 100644
--- a/arch/arm64/include/asm/pointer_auth.h
+++ b/arch/arm64/include/asm/pointer_auth.h
@@ -30,17 +30,23 @@ struct ptrauth_keys {
 	struct ptrauth_key apga;
 };
 
+static inline void __ptrauth_key_init(struct ptrauth_key *key)
+{
+	key->lo = get_random_u64();
+	key->hi = get_random_u64();
+}
+
 static inline void ptrauth_keys_init(struct ptrauth_keys *keys)
 {
 	if (system_supports_address_auth()) {
-		get_random_bytes(&keys->apia, sizeof(keys->apia));
-		get_random_bytes(&keys->apib, sizeof(keys->apib));
-		get_random_bytes(&keys->apda, sizeof(keys->apda));
-		get_random_bytes(&keys->apdb, sizeof(keys->apdb));
+		__ptrauth_key_init(&keys->apia);
+		__ptrauth_key_init(&keys->apib);
+		__ptrauth_key_init(&keys->apda);
+		__ptrauth_key_init(&keys->apdb);
 	}
 
 	if (system_supports_generic_auth())
-		get_random_bytes(&keys->apga, sizeof(keys->apga));
+		__ptrauth_key_init(&keys->apga);
 }
 
 #define __ptrauth_key_install(k, v)				\
diff --git a/arch/arm64/kernel/pointer_auth.c b/arch/arm64/kernel/pointer_auth.c
index c507b584259d0..05e2e3d174010 100644
--- a/arch/arm64/kernel/pointer_auth.c
+++ b/arch/arm64/kernel/pointer_auth.c
@@ -31,15 +31,15 @@ int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg)
 		return -EINVAL;
 
 	if (arg & PR_PAC_APIAKEY)
-		get_random_bytes(&keys->apia, sizeof(keys->apia));
+		__ptrauth_key_init(&keys->apia);
 	if (arg & PR_PAC_APIBKEY)
-		get_random_bytes(&keys->apib, sizeof(keys->apib));
+		__ptrauth_key_init(&keys->apib);
 	if (arg & PR_PAC_APDAKEY)
-		get_random_bytes(&keys->apda, sizeof(keys->apda));
+		__ptrauth_key_init(&keys->apda);
 	if (arg & PR_PAC_APDBKEY)
-		get_random_bytes(&keys->apdb, sizeof(keys->apdb));
+		__ptrauth_key_init(&keys->apdb);
 	if (arg & PR_PAC_APGAKEY)
-		get_random_bytes(&keys->apga, sizeof(keys->apga));
+		__ptrauth_key_init(&keys->apga);
 
 	ptrauth_keys_switch(keys);
 
-- 
2.26.0

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

Re: [PATCH v2] arm64: ptr auth: Use get_random_u64 instead of _bytes

[Mark Rutland]

This seems to have been sent twice, so I'm replying to the latest
copy.

On Tue, Mar 31, 2020 at 03:44:12PM +0000, George Spelvin wrote:
> get_random_bytes() is approximately 4x the cost of two
> get_random_u64() calls, because the former implements
> anti-backtracking.
> 
> Because these are authentication keys, useless to an attacker
> as soon as the kernel stops using them, there is no security
> benefit from anti-backtracking.
> 
> Signed-off-by: George Spelvin <lkml@sdf.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org

Given my understanding from discussion on the prior version the
conversion is sound, it doesn't matter that we expose the keys via
ptrace, and the code looks nicer, so:

Acked-by: Mark Rutland <mark.rutland@arm.com>

I assume Will or Catalin will pick this up when they next pick up
patches.

Mark.

> ---
> v2: Took out all the clever bitmap-based stuff and made a simple
>     boring helper function to replace get_random_bytes(&key, 16).
> 
>  arch/arm64/include/asm/pointer_auth.h | 16 +++++++++++-----
>  arch/arm64/kernel/pointer_auth.c      | 10 +++++-----
>  2 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> index 7a24bad1a58b8..1a4590d05785e 100644
> --- a/arch/arm64/include/asm/pointer_auth.h
> +++ b/arch/arm64/include/asm/pointer_auth.h
> @@ -30,17 +30,23 @@ struct ptrauth_keys {
>  	struct ptrauth_key apga;
>  };
>  
> +static inline void __ptrauth_key_init(struct ptrauth_key *key)
> +{
> +	key->lo = get_random_u64();
> +	key->hi = get_random_u64();
> +}
> +
>  static inline void ptrauth_keys_init(struct ptrauth_keys *keys)
>  {
>  	if (system_supports_address_auth()) {
> -		get_random_bytes(&keys->apia, sizeof(keys->apia));
> -		get_random_bytes(&keys->apib, sizeof(keys->apib));
> -		get_random_bytes(&keys->apda, sizeof(keys->apda));
> -		get_random_bytes(&keys->apdb, sizeof(keys->apdb));
> +		__ptrauth_key_init(&keys->apia);
> +		__ptrauth_key_init(&keys->apib);
> +		__ptrauth_key_init(&keys->apda);
> +		__ptrauth_key_init(&keys->apdb);
>  	}
>  
>  	if (system_supports_generic_auth())
> -		get_random_bytes(&keys->apga, sizeof(keys->apga));
> +		__ptrauth_key_init(&keys->apga);
>  }
>  
>  #define __ptrauth_key_install(k, v)				\
> diff --git a/arch/arm64/kernel/pointer_auth.c b/arch/arm64/kernel/pointer_auth.c
> index c507b584259d0..05e2e3d174010 100644
> --- a/arch/arm64/kernel/pointer_auth.c
> +++ b/arch/arm64/kernel/pointer_auth.c
> @@ -31,15 +31,15 @@ int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg)
>  		return -EINVAL;
>  
>  	if (arg & PR_PAC_APIAKEY)
> -		get_random_bytes(&keys->apia, sizeof(keys->apia));
> +		__ptrauth_key_init(&keys->apia);
>  	if (arg & PR_PAC_APIBKEY)
> -		get_random_bytes(&keys->apib, sizeof(keys->apib));
> +		__ptrauth_key_init(&keys->apib);
>  	if (arg & PR_PAC_APDAKEY)
> -		get_random_bytes(&keys->apda, sizeof(keys->apda));
> +		__ptrauth_key_init(&keys->apda);
>  	if (arg & PR_PAC_APDBKEY)
> -		get_random_bytes(&keys->apdb, sizeof(keys->apdb));
> +		__ptrauth_key_init(&keys->apdb);
>  	if (arg & PR_PAC_APGAKEY)
> -		get_random_bytes(&keys->apga, sizeof(keys->apga));
> +		__ptrauth_key_init(&keys->apga);
>  
>  	ptrauth_keys_switch(keys);
>  
> -- 
> 2.26.0

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

Re: [PATCH v2] arm64: ptr auth: Use get_random_u64 instead of _bytes

[Will Deacon]

Hi George,

On Tue, Mar 31, 2020 at 03:44:12PM +0000, George Spelvin wrote:
> get_random_bytes() is approximately 4x the cost of two
> get_random_u64() calls, because the former implements
> anti-backtracking.
> 
> Because these are authentication keys, useless to an attacker
> as soon as the kernel stops using them, there is no security
> benefit from anti-backtracking.
> 
> Signed-off-by: George Spelvin <lkml@sdf.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> v2: Took out all the clever bitmap-based stuff and made a simple
>     boring helper function to replace get_random_bytes(&key, 16).
> 
>  arch/arm64/include/asm/pointer_auth.h | 16 +++++++++++-----
>  arch/arm64/kernel/pointer_auth.c      | 10 +++++-----
>  2 files changed, 16 insertions(+), 10 deletions(-)

Please can you resend this against the arm64 for-next/ptr-auth branch [1]?
I can't apply it as-is.

Thanks,

Will

[1] https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=for-next/ptr-auth

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

Re: [PATCH v2] arm64: ptr auth: Use get_random_u64 instead of _bytes

[Will Deacon]

On Tue, Apr 28, 2020 at 01:58:12PM +0100, Will Deacon wrote:
> On Tue, Mar 31, 2020 at 03:44:12PM +0000, George Spelvin wrote:
> > get_random_bytes() is approximately 4x the cost of two
> > get_random_u64() calls, because the former implements
> > anti-backtracking.
> > 
> > Because these are authentication keys, useless to an attacker
> > as soon as the kernel stops using them, there is no security
> > benefit from anti-backtracking.
> > 
> > Signed-off-by: George Spelvin <lkml@sdf.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > ---
> > v2: Took out all the clever bitmap-based stuff and made a simple
> >     boring helper function to replace get_random_bytes(&key, 16).
> > 
> >  arch/arm64/include/asm/pointer_auth.h | 16 +++++++++++-----
> >  arch/arm64/kernel/pointer_auth.c      | 10 +++++-----
> >  2 files changed, 16 insertions(+), 10 deletions(-)
> 
> Please can you resend this against the arm64 for-next/ptr-auth branch [1]?
> I can't apply it as-is.

Any update on this one?

Cheers,

Will

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=for-next/ptr-auth

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