linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64/sve: Better handle failure to allocate SVE register storage
@ 2021-08-17 17:13 Mark Brown
  2021-08-24 10:32 ` Catalin Marinas
  0 siblings, 1 reply; 3+ messages in thread
From: Mark Brown @ 2021-08-17 17:13 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: linux-arm-kernel, Mark Brown

Currently we "handle" failure to allocate the SVE register storage by
doing a BUG_ON() and hoping for the best. This is obviously not great and
the memory allocation failure will already be loud enough without the
BUG_ON(). As the comment says it is a corner case but let's try to do a bit
better, remove the BUG_ON() and add code to handle the failure in the
callers.

For the ptrace and signal code we can return -ENOMEM gracefully however
we have no real error reporting path available to us for the SVE access
trap so instead generate a SIGKILL if the allocation fails there. This
at least means that we won't try to soldier on and end up trying to
access the nonexistant state and while it's obviously not ideal for
userspace SIGKILL doesn't allow any handling so minimises the ABI
impact, making it easier to improve the interface later if we come up
with a better idea.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kernel/fpsimd.c | 10 ++++------
 arch/arm64/kernel/ptrace.c |  4 ++++
 arch/arm64/kernel/signal.c |  3 +++
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index eb8d972ad3d2..5a294f20e9de 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -520,12 +520,6 @@ void sve_alloc(struct task_struct *task)
 	/* This is a small allocation (maximum ~8KB) and Should Not Fail. */
 	task->thread.sve_state =
 		kzalloc(sve_state_size(task), GFP_KERNEL);
-
-	/*
-	 * If future SVE revisions can have larger vectors though,
-	 * this may cease to be true:
-	 */
-	BUG_ON(!task->thread.sve_state);
 }
 
 
@@ -945,6 +939,10 @@ void do_sve_acc(unsigned int esr, struct pt_regs *regs)
 	}
 
 	sve_alloc(current);
+	if (!current->thread.sve_state) {
+		force_sig(SIGKILL);
+		return;
+	}
 
 	get_cpu_fpsimd_context();
 
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index b381a1ee9ea7..8bb3376b53fe 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -845,6 +845,10 @@ static int sve_set(struct task_struct *target,
 	}
 
 	sve_alloc(target);
+	if (!target->thread.sve_state) {
+		ret = -ENOMEM;
+		goto out;
+	}
 
 	/*
 	 * Ensure target->thread.sve_state is up to date with target's
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index aa46168bff32..34ea71ea6126 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -290,6 +290,9 @@ static int restore_sve_fpsimd_context(struct user_ctxs *user)
 	/* From now, fpsimd_thread_switch() won't touch thread.sve_state */
 
 	sve_alloc(current);
+	if (!current->thread.sve_state)
+		return -ENOMEM;
+
 	err = __copy_from_user(current->thread.sve_state,
 			       (char __user const *)user->sve +
 					SVE_SIG_REGS_OFFSET,
-- 
2.20.1


_______________________________________________
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] 3+ messages in thread

* Re: [PATCH] arm64/sve: Better handle failure to allocate SVE register storage
  2021-08-17 17:13 [PATCH] arm64/sve: Better handle failure to allocate SVE register storage Mark Brown
@ 2021-08-24 10:32 ` Catalin Marinas
  2021-08-24 12:48   ` Mark Brown
  0 siblings, 1 reply; 3+ messages in thread
From: Catalin Marinas @ 2021-08-24 10:32 UTC (permalink / raw)
  To: Mark Brown; +Cc: Will Deacon, linux-arm-kernel

On Tue, Aug 17, 2021 at 06:13:30PM +0100, Mark Brown wrote:
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index eb8d972ad3d2..5a294f20e9de 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -520,12 +520,6 @@ void sve_alloc(struct task_struct *task)
>  	/* This is a small allocation (maximum ~8KB) and Should Not Fail. */
>  	task->thread.sve_state =
>  		kzalloc(sve_state_size(task), GFP_KERNEL);
> -
> -	/*
> -	 * If future SVE revisions can have larger vectors though,
> -	 * this may cease to be true:
> -	 */
> -	BUG_ON(!task->thread.sve_state);
>  }

I think we should keep a WARN_ON_ONCE here in case one wonders why the
app was suddenly killed.

-- 
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] 3+ messages in thread

* Re: [PATCH] arm64/sve: Better handle failure to allocate SVE register storage
  2021-08-24 10:32 ` Catalin Marinas
@ 2021-08-24 12:48   ` Mark Brown
  0 siblings, 0 replies; 3+ messages in thread
From: Mark Brown @ 2021-08-24 12:48 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Will Deacon, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 471 bytes --]

On Tue, Aug 24, 2021 at 11:32:14AM +0100, Catalin Marinas wrote:
> On Tue, Aug 17, 2021 at 06:13:30PM +0100, Mark Brown wrote:

> > -	BUG_ON(!task->thread.sve_state);

> I think we should keep a WARN_ON_ONCE here in case one wonders why the
> app was suddenly killed.

OOM is very noisy already, the general advice is not to issue any
extra diagnostics on allocation failures as there's already going to be
at the very least a backtrace from the allocation failure site.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
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] 3+ messages in thread

end of thread, other threads:[~2021-08-24 12:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-17 17:13 [PATCH] arm64/sve: Better handle failure to allocate SVE register storage Mark Brown
2021-08-24 10:32 ` Catalin Marinas
2021-08-24 12:48   ` Mark Brown

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