[RFC PATCH v2 0/6] Signal frame expansion support

[RFC PATCH v2 0/6] Signal frame expansion support

[Dave Martin]

(Note: This is an arm64-specific series, but the concepts introduced may
be of interest to other arches -- see in particular patch 6.)

Blurb:

An architecture advertises the maximum possible signal frame size via
the MINSIGSTKSZ #define (mandated by POSIX).

However, CPU architecture extensions may increase the amount of space
required to store the interrupted context when a signal is delivered.


Eventually the amount of space needed in the signal frame may exceed
MINSIGSTKSZ -- whether and when this happens is largely a matter of
luck, depending on the initial guess for MINSIGSTKSZ and the evolution
of that particular CPU architecture.  Unfortunately MINSIGSTKSZ cannot
be changed without an ABI break, and POSIX provides no mechanism for
migration.

arm64 initially reserved 4KB of space in the signal frame for
extensions, of which about 0.5KB is allocated to the FP/SIMD registers
initially.

Depending on the vector length supported by the hardware, SVE requires
up to around 8KB of space to store the full SIMD register context, which
is too large to fit in the existing frame.

This series adds a mechanism for optionally enlarging the signal frame
(patches 4-5) and reporting the actual maximum signal frame size to
userspace (patch 6).  Patches 1-3 do some refactoring to support this
change by abstracting the way signal frame records are allocated onto
the user stack.

Full backwards compatibility is not possible -- there is no way to hide
the fact that the signal frame has grown -- so it is expected that
support for new architecture extensions that can cause the signal frame
to grow will be opt-in for userspace, in addition to using the extension
mechanism defined by this series.

[1] ARM Scalable Vector Extension
https://community.arm.com/groups/processors/blog/2016/08/22/technology-update-the-scalable-vector-extension-sve-for-the-armv8-a-architecture
https://developer.arm.com/docs/ddi0584/latest/arm-architecture-reference-manual-supplement-the-scalable-vector-extension-sve-for-armv8-a

Dave Martin (6):
  arm64: signal: Refactor sigcontext parsing in rt_sigreturn
  arm64: signal: factor frame layout and population into separate passes
  arm64: signal: factor out signal frame record allocation
  arm64: signal: Allocate extra sigcontext space as needed
  arm64: signal: Parse extra_context during sigreturn
  arm64: signal: Report signal frame size to userspace via auxv

 arch/arm64/include/asm/elf.h             |   5 +
 arch/arm64/include/asm/processor.h       |   3 +
 arch/arm64/include/uapi/asm/auxvec.h     |   3 +-
 arch/arm64/include/uapi/asm/sigcontext.h |  50 ++++
 arch/arm64/kernel/signal.c               | 389 ++++++++++++++++++++++++++++---
 5 files changed, 415 insertions(+), 35 deletions(-)

-- 
2.1.4

[RFC PATCH v2 0/6] Signal frame expansion support

[Dave Martin]

(Note: This is an arm64-specific series, but the concepts introduced may
be of interest to other arches -- see in particular patch 6.)

Blurb:

An architecture advertises the maximum possible signal frame size via
the MINSIGSTKSZ #define (mandated by POSIX).

However, CPU architecture extensions may increase the amount of space
required to store the interrupted context when a signal is delivered.


Eventually the amount of space needed in the signal frame may exceed
MINSIGSTKSZ -- whether and when this happens is largely a matter of
luck, depending on the initial guess for MINSIGSTKSZ and the evolution
of that particular CPU architecture.  Unfortunately MINSIGSTKSZ cannot
be changed without an ABI break, and POSIX provides no mechanism for
migration.

arm64 initially reserved 4KB of space in the signal frame for
extensions, of which about 0.5KB is allocated to the FP/SIMD registers
initially.

Depending on the vector length supported by the hardware, SVE requires
up to around 8KB of space to store the full SIMD register context, which
is too large to fit in the existing frame.

This series adds a mechanism for optionally enlarging the signal frame
(patches 4-5) and reporting the actual maximum signal frame size to
userspace (patch 6).  Patches 1-3 do some refactoring to support this
change by abstracting the way signal frame records are allocated onto
the user stack.

Full backwards compatibility is not possible -- there is no way to hide
the fact that the signal frame has grown -- so it is expected that
support for new architecture extensions that can cause the signal frame
to grow will be opt-in for userspace, in addition to using the extension
mechanism defined by this series.

[1] ARM Scalable Vector Extension
https://community.arm.com/groups/processors/blog/2016/08/22/technology-update-the-scalable-vector-extension-sve-for-the-armv8-a-architecture
https://developer.arm.com/docs/ddi0584/latest/arm-architecture-reference-manual-supplement-the-scalable-vector-extension-sve-for-armv8-a

Dave Martin (6):
  arm64: signal: Refactor sigcontext parsing in rt_sigreturn
  arm64: signal: factor frame layout and population into separate passes
  arm64: signal: factor out signal frame record allocation
  arm64: signal: Allocate extra sigcontext space as needed
  arm64: signal: Parse extra_context during sigreturn
  arm64: signal: Report signal frame size to userspace via auxv

 arch/arm64/include/asm/elf.h             |   5 +
 arch/arm64/include/asm/processor.h       |   3 +
 arch/arm64/include/uapi/asm/auxvec.h     |   3 +-
 arch/arm64/include/uapi/asm/sigcontext.h |  50 ++++
 arch/arm64/kernel/signal.c               | 389 ++++++++++++++++++++++++++++---
 5 files changed, 415 insertions(+), 35 deletions(-)

-- 
2.1.4

[RFC PATCH v2 1/6] arm64: signal: Refactor sigcontext parsing in rt_sigreturn

[Dave Martin]

Currently, rt_sigreturn does very limited checking on the
sigcontext coming from userspace.

Future additions of extra dynamic sigcontext data will increase the
potential for surprises.  Also, it is not clear whether the
sigcontext extension records are supposed to occur in a particular
order.

This patch factors out the sigcontext parsing into a separate
function, and adds extra checks to validate the well-formedness of
the sigcontext structure.

To help with this, an abstraction for the signal frame layout is
also added, using offsets to track the location of different
records in the frame.  Although trivial, this provides a base to
extend upon in order to track more complex layouts.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/kernel/signal.c | 121 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 101 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index afed8f5..d4c5e3e 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -19,9 +19,11 @@
 
 #include <linux/compat.h>
 #include <linux/errno.h>
+#include <linux/kernel.h>
 #include <linux/signal.h>
 #include <linux/personality.h>
 #include <linux/freezer.h>
+#include <linux/stddef.h>
 #include <linux/uaccess.h>
 #include <linux/tracehook.h>
 #include <linux/ratelimit.h>
@@ -45,6 +47,10 @@ struct rt_sigframe {
 	u64 lr;
 };
 
+struct rt_sigframe_user_layout {
+	struct rt_sigframe __user *sigframe;
+};
+
 static int preserve_fpsimd_context(struct fpsimd_context __user *ctx)
 {
 	struct fpsimd_state *fpsimd = &current->thread.fpsimd_state;
@@ -92,12 +98,86 @@ static int restore_fpsimd_context(struct fpsimd_context __user *ctx)
 	return err ? -EFAULT : 0;
 }
 
+struct user_ctxs {
+	struct fpsimd_context __user *fpsimd;
+};
+
+static int parse_user_sigframe(struct user_ctxs *user,
+			       struct rt_sigframe __user *sf)
+{
+	struct sigcontext __user *sc = &sf->uc.uc_mcontext;
+	struct _aarch64_ctx __user *head =
+		(struct _aarch64_ctx __user *)&sc->__reserved;
+	size_t offset = 0;
+
+	user->fpsimd = NULL;
+
+	while (1) {
+		int err;
+		u32 magic, size;
+
+		head = (struct _aarch64_ctx __user *)&sc->__reserved[offset];
+		if (!IS_ALIGNED((unsigned long)head, 16))
+			goto invalid;
+
+		err = 0;
+		__get_user_error(magic, &head->magic, err);
+		__get_user_error(size, &head->size, err);
+		if (err)
+			return err;
+
+		switch (magic) {
+		case 0:
+			if (size)
+				goto invalid;
+
+			goto done;
+
+		case FPSIMD_MAGIC:
+			if (user->fpsimd)
+				goto invalid;
+
+			if (offset > sizeof(sc->__reserved) -
+					sizeof(*user->fpsimd) ||
+			    size < sizeof(*user->fpsimd))
+				goto invalid;
+
+			user->fpsimd = (struct fpsimd_context __user *)head;
+			break;
+
+		case ESR_MAGIC:
+			/* ignore */
+			break;
+
+		default:
+			goto invalid;
+		}
+
+		if (size < sizeof(*head))
+			goto invalid;
+
+		if (size > sizeof(sc->__reserved) - (sizeof(*head) + offset))
+			goto invalid;
+
+		offset += size;
+	}
+
+done:
+	if (!user->fpsimd)
+		goto invalid;
+
+	return 0;
+
+invalid:
+	return -EINVAL;
+}
+
 static int restore_sigframe(struct pt_regs *regs,
 			    struct rt_sigframe __user *sf)
 {
 	sigset_t set;
 	int i, err;
-	void *aux = sf->uc.uc_mcontext.__reserved;
+	struct user_ctxs user;
 
 	err = __copy_from_user(&set, &sf->uc.uc_sigmask, sizeof(set));
 	if (err == 0)
@@ -116,12 +196,11 @@ static int restore_sigframe(struct pt_regs *regs,
 	zap_syscall(regs);
 
 	err |= !valid_user_regs(&regs->user_regs, current);
+	if (err == 0)
+		err = parse_user_sigframe(&user, sf);
 
-	if (err == 0) {
-		struct fpsimd_context *fpsimd_ctx =
-			container_of(aux, struct fpsimd_context, head);
-		err |= restore_fpsimd_context(fpsimd_ctx);
-	}
+	if (err == 0)
+		err = restore_fpsimd_context(user.fpsimd);
 
 	return err;
 }
@@ -162,10 +241,11 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 	return 0;
 }
 
-static int setup_sigframe(struct rt_sigframe __user *sf,
+static int setup_sigframe(struct rt_sigframe_user_layout *user,
 			  struct pt_regs *regs, sigset_t *set)
 {
 	int i, err = 0;
+	struct rt_sigframe __user *sf = user->sigframe;
 	void *aux = sf->uc.uc_mcontext.__reserved;
 	struct _aarch64_ctx *end;
 
@@ -209,33 +289,32 @@ static int setup_sigframe(struct rt_sigframe __user *sf,
 	return err;
 }
 
-static struct rt_sigframe __user *get_sigframe(struct ksignal *ksig,
-					       struct pt_regs *regs)
+static int get_sigframe(struct rt_sigframe_user_layout *user,
+			 struct ksignal *ksig, struct pt_regs *regs)
 {
 	unsigned long sp, sp_top;
-	struct rt_sigframe __user *frame;
 
 	sp = sp_top = sigsp(regs->sp, ksig);
 
 	sp = (sp - sizeof(struct rt_sigframe)) & ~15;
-	frame = (struct rt_sigframe __user *)sp;
+	user->sigframe = (struct rt_sigframe __user *)sp;
 
 	/*
 	 * Check that we can actually write to the signal frame.
 	 */
-	if (!access_ok(VERIFY_WRITE, frame, sp_top - sp))
-		frame = NULL;
+	if (!access_ok(VERIFY_WRITE, user->sigframe, sp_top - sp))
+		return -EFAULT;
 
-	return frame;
+	return 0;
 }
 
 static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
-			 void __user *frame, int usig)
+			 struct rt_sigframe_user_layout *user, int usig)
 {
 	__sigrestore_t sigtramp;
 
 	regs->regs[0] = usig;
-	regs->sp = (unsigned long)frame;
+	regs->sp = (unsigned long)user->sigframe;
 	regs->regs[29] = regs->sp + offsetof(struct rt_sigframe, fp);
 	regs->pc = (unsigned long)ka->sa.sa_handler;
 
@@ -250,20 +329,22 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
 static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
 			  struct pt_regs *regs)
 {
+	struct rt_sigframe_user_layout user;
 	struct rt_sigframe __user *frame;
 	int err = 0;
 
-	frame = get_sigframe(ksig, regs);
-	if (!frame)
+	if (get_sigframe(&user, ksig, regs))
 		return 1;
 
+	frame = user.sigframe;
+
 	__put_user_error(0, &frame->uc.uc_flags, err);
 	__put_user_error(NULL, &frame->uc.uc_link, err);
 
 	err |= __save_altstack(&frame->uc.uc_stack, regs->sp);
-	err |= setup_sigframe(frame, regs, set);
+	err |= setup_sigframe(&user, regs, set);
 	if (err == 0) {
-		setup_return(regs, &ksig->ka, frame, usig);
+		setup_return(regs, &ksig->ka, &user, usig);
 		if (ksig->ka.sa.sa_flags & SA_SIGINFO) {
 			err |= copy_siginfo_to_user(&frame->info, &ksig->info);
 			regs->regs[1] = (unsigned long)&frame->info;
-- 
2.1.4

[RFC PATCH v2 1/6] arm64: signal: Refactor sigcontext parsing in rt_sigreturn

[Dave Martin]

Currently, rt_sigreturn does very limited checking on the
sigcontext coming from userspace.

Future additions of extra dynamic sigcontext data will increase the
potential for surprises.  Also, it is not clear whether the
sigcontext extension records are supposed to occur in a particular
order.

This patch factors out the sigcontext parsing into a separate
function, and adds extra checks to validate the well-formedness of
the sigcontext structure.

To help with this, an abstraction for the signal frame layout is
also added, using offsets to track the location of different
records in the frame.  Although trivial, this provides a base to
extend upon in order to track more complex layouts.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/kernel/signal.c | 121 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 101 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index afed8f5..d4c5e3e 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -19,9 +19,11 @@
 
 #include <linux/compat.h>
 #include <linux/errno.h>
+#include <linux/kernel.h>
 #include <linux/signal.h>
 #include <linux/personality.h>
 #include <linux/freezer.h>
+#include <linux/stddef.h>
 #include <linux/uaccess.h>
 #include <linux/tracehook.h>
 #include <linux/ratelimit.h>
@@ -45,6 +47,10 @@ struct rt_sigframe {
 	u64 lr;
 };
 
+struct rt_sigframe_user_layout {
+	struct rt_sigframe __user *sigframe;
+};
+
 static int preserve_fpsimd_context(struct fpsimd_context __user *ctx)
 {
 	struct fpsimd_state *fpsimd = &current->thread.fpsimd_state;
@@ -92,12 +98,86 @@ static int restore_fpsimd_context(struct fpsimd_context __user *ctx)
 	return err ? -EFAULT : 0;
 }
 
+struct user_ctxs {
+	struct fpsimd_context __user *fpsimd;
+};
+
+static int parse_user_sigframe(struct user_ctxs *user,
+			       struct rt_sigframe __user *sf)
+{
+	struct sigcontext __user *sc = &sf->uc.uc_mcontext;
+	struct _aarch64_ctx __user *head =
+		(struct _aarch64_ctx __user *)&sc->__reserved;
+	size_t offset = 0;
+
+	user->fpsimd = NULL;
+
+	while (1) {
+		int err;
+		u32 magic, size;
+
+		head = (struct _aarch64_ctx __user *)&sc->__reserved[offset];
+		if (!IS_ALIGNED((unsigned long)head, 16))
+			goto invalid;
+
+		err = 0;
+		__get_user_error(magic, &head->magic, err);
+		__get_user_error(size, &head->size, err);
+		if (err)
+			return err;
+
+		switch (magic) {
+		case 0:
+			if (size)
+				goto invalid;
+
+			goto done;
+
+		case FPSIMD_MAGIC:
+			if (user->fpsimd)
+				goto invalid;
+
+			if (offset > sizeof(sc->__reserved) -
+					sizeof(*user->fpsimd) ||
+			    size < sizeof(*user->fpsimd))
+				goto invalid;
+
+			user->fpsimd = (struct fpsimd_context __user *)head;
+			break;
+
+		case ESR_MAGIC:
+			/* ignore */
+			break;
+
+		default:
+			goto invalid;
+		}
+
+		if (size < sizeof(*head))
+			goto invalid;
+
+		if (size > sizeof(sc->__reserved) - (sizeof(*head) + offset))
+			goto invalid;
+
+		offset += size;
+	}
+
+done:
+	if (!user->fpsimd)
+		goto invalid;
+
+	return 0;
+
+invalid:
+	return -EINVAL;
+}
+
 static int restore_sigframe(struct pt_regs *regs,
 			    struct rt_sigframe __user *sf)
 {
 	sigset_t set;
 	int i, err;
-	void *aux = sf->uc.uc_mcontext.__reserved;
+	struct user_ctxs user;
 
 	err = __copy_from_user(&set, &sf->uc.uc_sigmask, sizeof(set));
 	if (err == 0)
@@ -116,12 +196,11 @@ static int restore_sigframe(struct pt_regs *regs,
 	zap_syscall(regs);
 
 	err |= !valid_user_regs(&regs->user_regs, current);
+	if (err == 0)
+		err = parse_user_sigframe(&user, sf);
 
-	if (err == 0) {
-		struct fpsimd_context *fpsimd_ctx =
-			container_of(aux, struct fpsimd_context, head);
-		err |= restore_fpsimd_context(fpsimd_ctx);
-	}
+	if (err == 0)
+		err = restore_fpsimd_context(user.fpsimd);
 
 	return err;
 }
@@ -162,10 +241,11 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 	return 0;
 }
 
-static int setup_sigframe(struct rt_sigframe __user *sf,
+static int setup_sigframe(struct rt_sigframe_user_layout *user,
 			  struct pt_regs *regs, sigset_t *set)
 {
 	int i, err = 0;
+	struct rt_sigframe __user *sf = user->sigframe;
 	void *aux = sf->uc.uc_mcontext.__reserved;
 	struct _aarch64_ctx *end;
 
@@ -209,33 +289,32 @@ static int setup_sigframe(struct rt_sigframe __user *sf,
 	return err;
 }
 
-static struct rt_sigframe __user *get_sigframe(struct ksignal *ksig,
-					       struct pt_regs *regs)
+static int get_sigframe(struct rt_sigframe_user_layout *user,
+			 struct ksignal *ksig, struct pt_regs *regs)
 {
 	unsigned long sp, sp_top;
-	struct rt_sigframe __user *frame;
 
 	sp = sp_top = sigsp(regs->sp, ksig);
 
 	sp = (sp - sizeof(struct rt_sigframe)) & ~15;
-	frame = (struct rt_sigframe __user *)sp;
+	user->sigframe = (struct rt_sigframe __user *)sp;
 
 	/*
 	 * Check that we can actually write to the signal frame.
 	 */
-	if (!access_ok(VERIFY_WRITE, frame, sp_top - sp))
-		frame = NULL;
+	if (!access_ok(VERIFY_WRITE, user->sigframe, sp_top - sp))
+		return -EFAULT;
 
-	return frame;
+	return 0;
 }
 
 static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
-			 void __user *frame, int usig)
+			 struct rt_sigframe_user_layout *user, int usig)
 {
 	__sigrestore_t sigtramp;
 
 	regs->regs[0] = usig;
-	regs->sp = (unsigned long)frame;
+	regs->sp = (unsigned long)user->sigframe;
 	regs->regs[29] = regs->sp + offsetof(struct rt_sigframe, fp);
 	regs->pc = (unsigned long)ka->sa.sa_handler;
 
@@ -250,20 +329,22 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
 static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
 			  struct pt_regs *regs)
 {
+	struct rt_sigframe_user_layout user;
 	struct rt_sigframe __user *frame;
 	int err = 0;
 
-	frame = get_sigframe(ksig, regs);
-	if (!frame)
+	if (get_sigframe(&user, ksig, regs))
 		return 1;
 
+	frame = user.sigframe;
+
 	__put_user_error(0, &frame->uc.uc_flags, err);
 	__put_user_error(NULL, &frame->uc.uc_link, err);
 
 	err |= __save_altstack(&frame->uc.uc_stack, regs->sp);
-	err |= setup_sigframe(frame, regs, set);
+	err |= setup_sigframe(&user, regs, set);
 	if (err == 0) {
-		setup_return(regs, &ksig->ka, frame, usig);
+		setup_return(regs, &ksig->ka, &user, usig);
 		if (ksig->ka.sa.sa_flags & SA_SIGINFO) {
 			err |= copy_siginfo_to_user(&frame->info, &ksig->info);
 			regs->regs[1] = (unsigned long)&frame->info;
-- 
2.1.4

[RFC PATCH v2 2/6] arm64: signal: factor frame layout and population into separate passes

[Dave Martin]

In preparation for expanding the signal frame, this patch refactors
the signal frame setup code in setup_sigframe() into two separate
passes.

The first pass, setup_sigframe_layout(), determines the size of the
signal frame and its internal layout, including the presence and
location of optional records.  The resulting knowledge is used to
allocate and locate the user stack space required for the signal
frame and to determine which optional records to include.

The second pass, setup_sigframe(), is called once the stack frame
is allocated in order to populate it with the necessary context
information.

As a result of these changes, it becomes more natural to represent
locations in the signal frame by a base pointer and an offset,
since the absolute address of each location is not known during the
layout pass.  To be more consistent with this logic,
parse_user_sigframe() is refactored to describe signal frame
locations in the same way.  This enables a common representation
(struct rt_sigframe_user_layout) of the signal frame layout to be
used in all relevant code.

This change has no effect on the signal ABI, but will make it
easier to expand the signal frame in future patches.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/kernel/signal.c | 112 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 88 insertions(+), 24 deletions(-)

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index d4c5e3e..6202753 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -25,6 +25,7 @@
 #include <linux/freezer.h>
 #include <linux/stddef.h>
 #include <linux/uaccess.h>
+#include <linux/string.h>
 #include <linux/tracehook.h>
 #include <linux/ratelimit.h>
 
@@ -49,8 +50,39 @@ struct rt_sigframe {
 
 struct rt_sigframe_user_layout {
 	struct rt_sigframe __user *sigframe;
+
+	unsigned long size;	/* size of allocated sigframe data */
+	unsigned long limit;	/* largest allowed size */
+
+	unsigned long fpsimd_offset;
+	unsigned long esr_offset;
+	unsigned long end_offset;
 };
 
+static void init_user_layout(struct rt_sigframe_user_layout *user)
+{
+	memset(user, 0, sizeof(*user));
+	user->size = offsetof(struct rt_sigframe, uc.uc_mcontext.__reserved);
+
+	user->limit = user->size +
+		sizeof(user->sigframe->uc.uc_mcontext.__reserved) -
+		round_up(sizeof(struct _aarch64_ctx), 16);
+		/* ^ reserve space for terminator */
+}
+
+static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
+{
+	return round_up(max(user->size, sizeof(struct rt_sigframe)), 16);
+}
+
+static void __user *apply_user_offset(
+	struct rt_sigframe_user_layout const *user, unsigned long offset)
+{
+	char __user *base = (char __user *)user->sigframe;
+
+	return base + offset;
+}
+
 static int preserve_fpsimd_context(struct fpsimd_context __user *ctx)
 {
 	struct fpsimd_state *fpsimd = &current->thread.fpsimd_state;
@@ -106,26 +138,35 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			       struct rt_sigframe __user *sf)
 {
 	struct sigcontext __user *sc = &sf->uc.uc_mcontext;
-	struct _aarch64_ctx __user *head =
-		(struct _aarch64_ctx __user *)&sc->__reserved;
+	struct _aarch64_ctx __user *head;
+	char __user *base = (char __user *)&sc->__reserved;
 	size_t offset = 0;
+	size_t limit = sizeof(sc->__reserved);
 
 	user->fpsimd = NULL;
 
+	if (!IS_ALIGNED((unsigned long)base, 16))
+		goto invalid;
+
 	while (1) {
-		int err;
+		int err = 0;
 		u32 magic, size;
 
-		head = (struct _aarch64_ctx __user *)&sc->__reserved[offset];
-		if (!IS_ALIGNED((unsigned long)head, 16))
+		if (limit - offset < sizeof(*head))
 			goto invalid;
 
-		err = 0;
+		if (!IS_ALIGNED(offset, 16))
+			goto invalid;
+
+		head = (struct _aarch64_ctx __user *)(base + offset);
 		__get_user_error(magic, &head->magic, err);
 		__get_user_error(size, &head->size, err);
 		if (err)
 			return err;
 
+		if (limit - offset < size)
+			goto invalid;
+
 		switch (magic) {
 		case 0:
 			if (size)
@@ -137,9 +178,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			if (user->fpsimd)
 				goto invalid;
 
-			if (offset > sizeof(sc->__reserved) -
-					sizeof(*user->fpsimd) ||
-			    size < sizeof(*user->fpsimd))
+			if (size < sizeof(*user->fpsimd))
 				goto invalid;
 
 			user->fpsimd = (struct fpsimd_context __user *)head;
@@ -156,7 +195,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 		if (size < sizeof(*head))
 			goto invalid;
 
-		if (size > sizeof(sc->__reserved) - (sizeof(*head) + offset))
+		if (limit - offset < size)
 			goto invalid;
 
 		offset += size;
@@ -241,13 +280,30 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 	return 0;
 }
 
+/* Determine the layout of optional records in the signal frame */
+static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
+{
+	user->fpsimd_offset = user->size;
+	user->size += round_up(sizeof(struct fpsimd_context), 16);
+
+	/* fault information, if valid */
+	if (current->thread.fault_code) {
+		user->esr_offset = user->size;
+		user->size += round_up(sizeof(struct esr_context), 16);
+	}
+
+	/* set the "end" magic */
+	user->end_offset = user->size;
+
+	return 0;
+}
+
+
 static int setup_sigframe(struct rt_sigframe_user_layout *user,
 			  struct pt_regs *regs, sigset_t *set)
 {
 	int i, err = 0;
 	struct rt_sigframe __user *sf = user->sigframe;
-	void *aux = sf->uc.uc_mcontext.__reserved;
-	struct _aarch64_ctx *end;
 
 	/* set up the stack frame for unwinding */
 	__put_user_error(regs->regs[29], &sf->fp, err);
@@ -265,26 +321,29 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 	err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));
 
 	if (err == 0) {
-		struct fpsimd_context *fpsimd_ctx =
-			container_of(aux, struct fpsimd_context, head);
+		struct fpsimd_context __user *fpsimd_ctx =
+			apply_user_offset(user, user->fpsimd_offset);
 		err |= preserve_fpsimd_context(fpsimd_ctx);
-		aux += sizeof(*fpsimd_ctx);
 	}
 
 	/* fault information, if valid */
-	if (current->thread.fault_code) {
-		struct esr_context *esr_ctx =
-			container_of(aux, struct esr_context, head);
+	if (err == 0 && user->esr_offset) {
+		struct esr_context __user *esr_ctx =
+			apply_user_offset(user, user->esr_offset);
+
 		__put_user_error(ESR_MAGIC, &esr_ctx->head.magic, err);
 		__put_user_error(sizeof(*esr_ctx), &esr_ctx->head.size, err);
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
-		aux += sizeof(*esr_ctx);
 	}
 
 	/* set the "end" magic */
-	end = aux;
-	__put_user_error(0, &end->magic, err);
-	__put_user_error(0, &end->size, err);
+	if (err == 0) {
+		struct _aarch64_ctx __user *end =
+			apply_user_offset(user, user->end_offset);
+
+		__put_user_error(0, &end->magic, err);
+		__put_user_error(0, &end->size, err);
+	}
 
 	return err;
 }
@@ -293,10 +352,15 @@ static int get_sigframe(struct rt_sigframe_user_layout *user,
 			 struct ksignal *ksig, struct pt_regs *regs)
 {
 	unsigned long sp, sp_top;
+	int err;
 
-	sp = sp_top = sigsp(regs->sp, ksig);
+	init_user_layout(user);
+	err = setup_sigframe_layout(user);
+	if (err)
+		return err;
 
-	sp = (sp - sizeof(struct rt_sigframe)) & ~15;
+	sp = sp_top = sigsp(regs->sp, ksig);
+	sp = (sp & ~15) - sigframe_size(user);
 	user->sigframe = (struct rt_sigframe __user *)sp;
 
 	/*
-- 
2.1.4

[RFC PATCH v2 2/6] arm64: signal: factor frame layout and population into separate passes

[Dave Martin]

In preparation for expanding the signal frame, this patch refactors
the signal frame setup code in setup_sigframe() into two separate
passes.

The first pass, setup_sigframe_layout(), determines the size of the
signal frame and its internal layout, including the presence and
location of optional records.  The resulting knowledge is used to
allocate and locate the user stack space required for the signal
frame and to determine which optional records to include.

The second pass, setup_sigframe(), is called once the stack frame
is allocated in order to populate it with the necessary context
information.

As a result of these changes, it becomes more natural to represent
locations in the signal frame by a base pointer and an offset,
since the absolute address of each location is not known during the
layout pass.  To be more consistent with this logic,
parse_user_sigframe() is refactored to describe signal frame
locations in the same way.  This enables a common representation
(struct rt_sigframe_user_layout) of the signal frame layout to be
used in all relevant code.

This change has no effect on the signal ABI, but will make it
easier to expand the signal frame in future patches.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/kernel/signal.c | 112 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 88 insertions(+), 24 deletions(-)

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index d4c5e3e..6202753 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -25,6 +25,7 @@
 #include <linux/freezer.h>
 #include <linux/stddef.h>
 #include <linux/uaccess.h>
+#include <linux/string.h>
 #include <linux/tracehook.h>
 #include <linux/ratelimit.h>
 
@@ -49,8 +50,39 @@ struct rt_sigframe {
 
 struct rt_sigframe_user_layout {
 	struct rt_sigframe __user *sigframe;
+
+	unsigned long size;	/* size of allocated sigframe data */
+	unsigned long limit;	/* largest allowed size */
+
+	unsigned long fpsimd_offset;
+	unsigned long esr_offset;
+	unsigned long end_offset;
 };
 
+static void init_user_layout(struct rt_sigframe_user_layout *user)
+{
+	memset(user, 0, sizeof(*user));
+	user->size = offsetof(struct rt_sigframe, uc.uc_mcontext.__reserved);
+
+	user->limit = user->size +
+		sizeof(user->sigframe->uc.uc_mcontext.__reserved) -
+		round_up(sizeof(struct _aarch64_ctx), 16);
+		/* ^ reserve space for terminator */
+}
+
+static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
+{
+	return round_up(max(user->size, sizeof(struct rt_sigframe)), 16);
+}
+
+static void __user *apply_user_offset(
+	struct rt_sigframe_user_layout const *user, unsigned long offset)
+{
+	char __user *base = (char __user *)user->sigframe;
+
+	return base + offset;
+}
+
 static int preserve_fpsimd_context(struct fpsimd_context __user *ctx)
 {
 	struct fpsimd_state *fpsimd = &current->thread.fpsimd_state;
@@ -106,26 +138,35 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			       struct rt_sigframe __user *sf)
 {
 	struct sigcontext __user *sc = &sf->uc.uc_mcontext;
-	struct _aarch64_ctx __user *head =
-		(struct _aarch64_ctx __user *)&sc->__reserved;
+	struct _aarch64_ctx __user *head;
+	char __user *base = (char __user *)&sc->__reserved;
 	size_t offset = 0;
+	size_t limit = sizeof(sc->__reserved);
 
 	user->fpsimd = NULL;
 
+	if (!IS_ALIGNED((unsigned long)base, 16))
+		goto invalid;
+
 	while (1) {
-		int err;
+		int err = 0;
 		u32 magic, size;
 
-		head = (struct _aarch64_ctx __user *)&sc->__reserved[offset];
-		if (!IS_ALIGNED((unsigned long)head, 16))
+		if (limit - offset < sizeof(*head))
 			goto invalid;
 
-		err = 0;
+		if (!IS_ALIGNED(offset, 16))
+			goto invalid;
+
+		head = (struct _aarch64_ctx __user *)(base + offset);
 		__get_user_error(magic, &head->magic, err);
 		__get_user_error(size, &head->size, err);
 		if (err)
 			return err;
 
+		if (limit - offset < size)
+			goto invalid;
+
 		switch (magic) {
 		case 0:
 			if (size)
@@ -137,9 +178,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			if (user->fpsimd)
 				goto invalid;
 
-			if (offset > sizeof(sc->__reserved) -
-					sizeof(*user->fpsimd) ||
-			    size < sizeof(*user->fpsimd))
+			if (size < sizeof(*user->fpsimd))
 				goto invalid;
 
 			user->fpsimd = (struct fpsimd_context __user *)head;
@@ -156,7 +195,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 		if (size < sizeof(*head))
 			goto invalid;
 
-		if (size > sizeof(sc->__reserved) - (sizeof(*head) + offset))
+		if (limit - offset < size)
 			goto invalid;
 
 		offset += size;
@@ -241,13 +280,30 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 	return 0;
 }
 
+/* Determine the layout of optional records in the signal frame */
+static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
+{
+	user->fpsimd_offset = user->size;
+	user->size += round_up(sizeof(struct fpsimd_context), 16);
+
+	/* fault information, if valid */
+	if (current->thread.fault_code) {
+		user->esr_offset = user->size;
+		user->size += round_up(sizeof(struct esr_context), 16);
+	}
+
+	/* set the "end" magic */
+	user->end_offset = user->size;
+
+	return 0;
+}
+
+
 static int setup_sigframe(struct rt_sigframe_user_layout *user,
 			  struct pt_regs *regs, sigset_t *set)
 {
 	int i, err = 0;
 	struct rt_sigframe __user *sf = user->sigframe;
-	void *aux = sf->uc.uc_mcontext.__reserved;
-	struct _aarch64_ctx *end;
 
 	/* set up the stack frame for unwinding */
 	__put_user_error(regs->regs[29], &sf->fp, err);
@@ -265,26 +321,29 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 	err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));
 
 	if (err == 0) {
-		struct fpsimd_context *fpsimd_ctx =
-			container_of(aux, struct fpsimd_context, head);
+		struct fpsimd_context __user *fpsimd_ctx =
+			apply_user_offset(user, user->fpsimd_offset);
 		err |= preserve_fpsimd_context(fpsimd_ctx);
-		aux += sizeof(*fpsimd_ctx);
 	}
 
 	/* fault information, if valid */
-	if (current->thread.fault_code) {
-		struct esr_context *esr_ctx =
-			container_of(aux, struct esr_context, head);
+	if (err == 0 && user->esr_offset) {
+		struct esr_context __user *esr_ctx =
+			apply_user_offset(user, user->esr_offset);
+
 		__put_user_error(ESR_MAGIC, &esr_ctx->head.magic, err);
 		__put_user_error(sizeof(*esr_ctx), &esr_ctx->head.size, err);
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
-		aux += sizeof(*esr_ctx);
 	}
 
 	/* set the "end" magic */
-	end = aux;
-	__put_user_error(0, &end->magic, err);
-	__put_user_error(0, &end->size, err);
+	if (err == 0) {
+		struct _aarch64_ctx __user *end =
+			apply_user_offset(user, user->end_offset);
+
+		__put_user_error(0, &end->magic, err);
+		__put_user_error(0, &end->size, err);
+	}
 
 	return err;
 }
@@ -293,10 +352,15 @@ static int get_sigframe(struct rt_sigframe_user_layout *user,
 			 struct ksignal *ksig, struct pt_regs *regs)
 {
 	unsigned long sp, sp_top;
+	int err;
 
-	sp = sp_top = sigsp(regs->sp, ksig);
+	init_user_layout(user);
+	err = setup_sigframe_layout(user);
+	if (err)
+		return err;
 
-	sp = (sp - sizeof(struct rt_sigframe)) & ~15;
+	sp = sp_top = sigsp(regs->sp, ksig);
+	sp = (sp & ~15) - sigframe_size(user);
 	user->sigframe = (struct rt_sigframe __user *)sp;
 
 	/*
-- 
2.1.4

[RFC PATCH v2 3/6] arm64: signal: factor out signal frame record allocation

[Dave Martin]

This patch factors out the allocator for signal frame optional
records into a separate function, to ensure consistency and
facilitate later expansion of the signal frame, so that the
allocation mechanism can be elaborated more easily in future
patches.

No overrun checking is currently done, because the allocation is in
user memory, and because the kernel statically does not attempt to
allocate enough space in the signal frame yet for an overrun to
occur.  This behaviour will be refined in future patches.

The approach taken in this patch to allocation of the terminator
record is not very clean: this will also be replaced in subsequent
patches.

For future extension, a table is added documenting the current
static allocations in __reserved[].  This will be important for
determining under what circumstances userspace may or may not see
an expanded signal frame.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/include/uapi/asm/sigcontext.h | 19 ++++++++++++++
 arch/arm64/kernel/signal.c               | 43 ++++++++++++++++++++++++++------
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index ee469be..1328a2c 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h
@@ -34,6 +34,25 @@ struct sigcontext {
 };
 
 /*
+ * Allocation of __reserved[]:
+ * (Note: records do not necessarily occur in the order shown here.)
+ *
+ *	size		description
+ *
+ *	0x210		fpsimd_context
+ *	 0x10		esr_context
+ *	 0x10		terminator (null _aarch64_ctx)
+ *
+ *	0xdd0		(reserved for future allocation)
+ *
+ * New records that can exceed this space need to be opt-in for userspace, so
+ * that an expanded signal frame is not generated unexpectedly.  The mechanism
+ * for opting in will depend on the extension that generates each new record.
+ * The above table documents the maximum set and sizes of records than can be
+ * generated when userspace does not opt in for any such extension.
+ */
+
+/*
  * Header to be used at the beginning of structures extending the user
  * context. Such structures must be placed after the rt_sigframe on the stack
  * and be 16-byte aligned. The last structure must be a dummy one with the
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 6202753..47592af 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -75,6 +75,22 @@ static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
 	return round_up(max(user->size, sizeof(struct rt_sigframe)), 16);
 }
 
+/*
+ * Allocate space for an optional record of <size> bytes in the user
+ * signal frame.  The offset from the signal frame base address to the
+ * allocated block is assigned to *offset.
+ */
+static int sigframe_alloc(struct rt_sigframe_user_layout *user,
+			  unsigned long *offset, size_t size)
+{
+	size_t padded_size = round_up(size, 16);
+
+	*offset = user->size;
+	user->size += padded_size;
+
+	return 0;
+}
+
 static void __user *apply_user_offset(
 	struct rt_sigframe_user_layout const *user, unsigned long offset)
 {
@@ -283,19 +299,32 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 /* Determine the layout of optional records in the signal frame */
 static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
 {
-	user->fpsimd_offset = user->size;
-	user->size += round_up(sizeof(struct fpsimd_context), 16);
+	int err;
+
+	err = sigframe_alloc(user, &user->fpsimd_offset,
+			     sizeof(struct fpsimd_context));
+	if (err)
+		return err;
 
 	/* fault information, if valid */
 	if (current->thread.fault_code) {
-		user->esr_offset = user->size;
-		user->size += round_up(sizeof(struct esr_context), 16);
+		err = sigframe_alloc(user, &user->esr_offset,
+				     sizeof(struct esr_context));
+		if (err)
+			return err;
 	}
 
-	/* set the "end" magic */
-	user->end_offset = user->size;
+	/*
+	 * Allocate space for the terminator record.
+	 * HACK: here we undo the reservation of space for the end record.
+	 * This bodge should be replaced with a cleaner approach later on.
+	 */
+	user->limit = offsetof(struct rt_sigframe, uc.uc_mcontext.__reserved) +
+		sizeof(user->sigframe->uc.uc_mcontext.__reserved);
 
-	return 0;
+	err = sigframe_alloc(user, &user->end_offset,
+			     sizeof(struct _aarch64_ctx));
+	return err;
 }
 
 
-- 
2.1.4

[RFC PATCH v2 3/6] arm64: signal: factor out signal frame record allocation

[Dave Martin]

This patch factors out the allocator for signal frame optional
records into a separate function, to ensure consistency and
facilitate later expansion of the signal frame, so that the
allocation mechanism can be elaborated more easily in future
patches.

No overrun checking is currently done, because the allocation is in
user memory, and because the kernel statically does not attempt to
allocate enough space in the signal frame yet for an overrun to
occur.  This behaviour will be refined in future patches.

The approach taken in this patch to allocation of the terminator
record is not very clean: this will also be replaced in subsequent
patches.

For future extension, a table is added documenting the current
static allocations in __reserved[].  This will be important for
determining under what circumstances userspace may or may not see
an expanded signal frame.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/include/uapi/asm/sigcontext.h | 19 ++++++++++++++
 arch/arm64/kernel/signal.c               | 43 ++++++++++++++++++++++++++------
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index ee469be..1328a2c 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h
@@ -34,6 +34,25 @@ struct sigcontext {
 };
 
 /*
+ * Allocation of __reserved[]:
+ * (Note: records do not necessarily occur in the order shown here.)
+ *
+ *	size		description
+ *
+ *	0x210		fpsimd_context
+ *	 0x10		esr_context
+ *	 0x10		terminator (null _aarch64_ctx)
+ *
+ *	0xdd0		(reserved for future allocation)
+ *
+ * New records that can exceed this space need to be opt-in for userspace, so
+ * that an expanded signal frame is not generated unexpectedly.  The mechanism
+ * for opting in will depend on the extension that generates each new record.
+ * The above table documents the maximum set and sizes of records than can be
+ * generated when userspace does not opt in for any such extension.
+ */
+
+/*
  * Header to be used at the beginning of structures extending the user
  * context. Such structures must be placed after the rt_sigframe on the stack
  * and be 16-byte aligned. The last structure must be a dummy one with the
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 6202753..47592af 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -75,6 +75,22 @@ static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
 	return round_up(max(user->size, sizeof(struct rt_sigframe)), 16);
 }
 
+/*
+ * Allocate space for an optional record of <size> bytes in the user
+ * signal frame.  The offset from the signal frame base address to the
+ * allocated block is assigned to *offset.
+ */
+static int sigframe_alloc(struct rt_sigframe_user_layout *user,
+			  unsigned long *offset, size_t size)
+{
+	size_t padded_size = round_up(size, 16);
+
+	*offset = user->size;
+	user->size += padded_size;
+
+	return 0;
+}
+
 static void __user *apply_user_offset(
 	struct rt_sigframe_user_layout const *user, unsigned long offset)
 {
@@ -283,19 +299,32 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 /* Determine the layout of optional records in the signal frame */
 static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
 {
-	user->fpsimd_offset = user->size;
-	user->size += round_up(sizeof(struct fpsimd_context), 16);
+	int err;
+
+	err = sigframe_alloc(user, &user->fpsimd_offset,
+			     sizeof(struct fpsimd_context));
+	if (err)
+		return err;
 
 	/* fault information, if valid */
 	if (current->thread.fault_code) {
-		user->esr_offset = user->size;
-		user->size += round_up(sizeof(struct esr_context), 16);
+		err = sigframe_alloc(user, &user->esr_offset,
+				     sizeof(struct esr_context));
+		if (err)
+			return err;
 	}
 
-	/* set the "end" magic */
-	user->end_offset = user->size;
+	/*
+	 * Allocate space for the terminator record.
+	 * HACK: here we undo the reservation of space for the end record.
+	 * This bodge should be replaced with a cleaner approach later on.
+	 */
+	user->limit = offsetof(struct rt_sigframe, uc.uc_mcontext.__reserved) +
+		sizeof(user->sigframe->uc.uc_mcontext.__reserved);
 
-	return 0;
+	err = sigframe_alloc(user, &user->end_offset,
+			     sizeof(struct _aarch64_ctx));
+	return err;
 }
 
 
-- 
2.1.4

[RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Dave Martin]

This patch modifies the context block allocator to create an
extra_context expansion block as necessary, and adds the necessary
code to populate, parse and decode this block.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/include/uapi/asm/sigcontext.h |  27 ++++++++
 arch/arm64/kernel/signal.c               | 111 ++++++++++++++++++++++++++-----
 2 files changed, 120 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 1328a2c..b5e2523 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h
@@ -80,4 +80,31 @@ struct esr_context {
 	__u64 esr;
 };
 
+/*
+ * Pointer to extra space for additional structures that don't fit in
+ * sigcontext.__reserved[].  Note:
+ *
+ * 1) fpsimd_context, esr_context and extra_context must be placed in
+ * sigcontext.__reserved[] if present.  They cannot be placed in the
+ * extra space.  Any other record can be placed either in the extra
+ * space or in sigcontext.__reserved[].
+ *
+ * 2) There must not be more than one extra_context.
+ *
+ * 3) If extra_context is present, it must be followed immediately in
+ * sigcontext.__reserved[] by the terminating null _aarch64_ctx (i.e.,
+ * extra_context must be the last record in sigcontext.__reserved[]
+ * except for the terminator).
+ *
+ * 4) The extra space must itself be terminated with a null
+ * _aarch64_ctx.
+ */
+#define EXTRA_MAGIC	0x45585401
+
+struct extra_context {
+	struct _aarch64_ctx head;
+	void __user *data;	/* 16-byte aligned pointer to extra space */
+	__u32 size;		/* size in bytes of the extra space */
+};
+
 #endif /* _UAPI__ASM_SIGCONTEXT_H */
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 47592af..95547e1 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -25,6 +25,7 @@
 #include <linux/freezer.h>
 #include <linux/stddef.h>
 #include <linux/uaccess.h>
+#include <linux/sizes.h>
 #include <linux/string.h>
 #include <linux/tracehook.h>
 #include <linux/ratelimit.h>
@@ -56,18 +57,27 @@ struct rt_sigframe_user_layout {
 
 	unsigned long fpsimd_offset;
 	unsigned long esr_offset;
+	unsigned long extra_offset;
 	unsigned long end_offset;
 };
 
+#define BASE_SIGFRAME_SIZE round_up(sizeof(struct rt_sigframe), 16)
+#define TERMINATOR_SIZE round_up(sizeof(struct _aarch64_ctx), 16)
+#define EXTRA_CONTEXT_SIZE round_up(sizeof(struct extra_context), 16)
+
 static void init_user_layout(struct rt_sigframe_user_layout *user)
 {
+	const size_t reserved_size =
+		sizeof(user->sigframe->uc.uc_mcontext.__reserved);
+
 	memset(user, 0, sizeof(*user));
 	user->size = offsetof(struct rt_sigframe, uc.uc_mcontext.__reserved);
 
-	user->limit = user->size +
-		sizeof(user->sigframe->uc.uc_mcontext.__reserved) -
-		round_up(sizeof(struct _aarch64_ctx), 16);
-		/* ^ reserve space for terminator */
+	user->limit = user->size + reserved_size;
+
+	user->limit -= TERMINATOR_SIZE;
+	user->limit -= EXTRA_CONTEXT_SIZE;
+	/* Reserve space for extension and terminator ^ */
 }
 
 static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
@@ -75,6 +85,48 @@ static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
 	return round_up(max(user->size, sizeof(struct rt_sigframe)), 16);
 }
 
+/* Sanity limit on the maximum size of signal frame we'll try to generate. */
+/* This is NOT ABI. */
+#define SIGFRAME_MAXSZ SZ_64K
+
+static int __sigframe_alloc(struct rt_sigframe_user_layout *user,
+			    unsigned long *offset, size_t size, bool extend)
+{
+	size_t padded_size = round_up(size, 16);
+
+	if (padded_size > user->limit - user->size &&
+	    !user->extra_offset &&
+	    extend) {
+		int ret;
+
+		ret = __sigframe_alloc(user, &user->extra_offset,
+				       sizeof(struct extra_context), false);
+		if (ret)
+			return ret;
+
+		/*
+		 * Further allocations must go after the fixed-size
+		 * part of the signal frame:
+		 */
+		user->size = BASE_SIGFRAME_SIZE;
+
+		/*
+		 * Allow expansion up to SIGFRAME_MAXSZ, ensuring space for
+		 * the terminator:
+		 */
+		user->limit = SIGFRAME_MAXSZ - TERMINATOR_SIZE;
+	}
+
+	/* Still not enough space?  Bad luck! */
+	if (padded_size > user->limit - user->size)
+		return -ENOMEM;
+
+	*offset = user->size;
+	user->size += padded_size;
+
+	return 0;
+}
+
 /*
  * Allocate space for an optional record of <size> bytes in the user
  * signal frame.  The offset from the signal frame base address to the
@@ -83,11 +135,24 @@ static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
 static int sigframe_alloc(struct rt_sigframe_user_layout *user,
 			  unsigned long *offset, size_t size)
 {
-	size_t padded_size = round_up(size, 16);
+	return __sigframe_alloc(user, offset, size, true);
+}
 
-	*offset = user->size;
-	user->size += padded_size;
+/* Allocate the null terminator record and prevent further allocations */
+static int sigframe_alloc_end(struct rt_sigframe_user_layout *user)
+{
+	int ret;
+
+	/* Un-reserve the space reserved for the terminator: */
+	user->limit += TERMINATOR_SIZE;
+
+	ret = sigframe_alloc(user, &user->end_offset,
+			     sizeof(struct _aarch64_ctx));
+	if (ret)
+		return ret;
 
+	/* Prevent further allocation: */
+	user->limit = user->size;
 	return 0;
 }
 
@@ -314,17 +379,7 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
 			return err;
 	}
 
-	/*
-	 * Allocate space for the terminator record.
-	 * HACK: here we undo the reservation of space for the end record.
-	 * This bodge should be replaced with a cleaner approach later on.
-	 */
-	user->limit = offsetof(struct rt_sigframe, uc.uc_mcontext.__reserved) +
-		sizeof(user->sigframe->uc.uc_mcontext.__reserved);
-
-	err = sigframe_alloc(user, &user->end_offset,
-			     sizeof(struct _aarch64_ctx));
-	return err;
+	return sigframe_alloc_end(user);
 }
 
 
@@ -365,6 +420,26 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
 	}
 
+	if (err == 0 && user->extra_offset) {
+		struct extra_context __user *extra =
+			apply_user_offset(user, user->extra_offset);
+		struct _aarch64_ctx __user *end =
+			(struct _aarch64_ctx __user *)((char __user *)extra +
+				round_up(sizeof(*extra), 16));
+		void __user *extra_data =
+			apply_user_offset(user, BASE_SIGFRAME_SIZE);
+		u32 extra_size = round_up(user->size, 16) - BASE_SIGFRAME_SIZE;
+
+		__put_user_error(EXTRA_MAGIC, &extra->head.magic, err);
+		__put_user_error(sizeof(*extra), &extra->head.size, err);
+		__put_user_error(extra_data, &extra->data, err);
+		__put_user_error(extra_size, &extra->size, err);
+
+		/* Add the terminator */
+		__put_user_error(0, &end->magic, err);
+		__put_user_error(0, &end->size, err);
+	}
+
 	/* set the "end" magic */
 	if (err == 0) {
 		struct _aarch64_ctx __user *end =
-- 
2.1.4

[RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Dave Martin]

This patch modifies the context block allocator to create an
extra_context expansion block as necessary, and adds the necessary
code to populate, parse and decode this block.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/include/uapi/asm/sigcontext.h |  27 ++++++++
 arch/arm64/kernel/signal.c               | 111 ++++++++++++++++++++++++++-----
 2 files changed, 120 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 1328a2c..b5e2523 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h
@@ -80,4 +80,31 @@ struct esr_context {
 	__u64 esr;
 };
 
+/*
+ * Pointer to extra space for additional structures that don't fit in
+ * sigcontext.__reserved[].  Note:
+ *
+ * 1) fpsimd_context, esr_context and extra_context must be placed in
+ * sigcontext.__reserved[] if present.  They cannot be placed in the
+ * extra space.  Any other record can be placed either in the extra
+ * space or in sigcontext.__reserved[].
+ *
+ * 2) There must not be more than one extra_context.
+ *
+ * 3) If extra_context is present, it must be followed immediately in
+ * sigcontext.__reserved[] by the terminating null _aarch64_ctx (i.e.,
+ * extra_context must be the last record in sigcontext.__reserved[]
+ * except for the terminator).
+ *
+ * 4) The extra space must itself be terminated with a null
+ * _aarch64_ctx.
+ */
+#define EXTRA_MAGIC	0x45585401
+
+struct extra_context {
+	struct _aarch64_ctx head;
+	void __user *data;	/* 16-byte aligned pointer to extra space */
+	__u32 size;		/* size in bytes of the extra space */
+};
+
 #endif /* _UAPI__ASM_SIGCONTEXT_H */
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 47592af..95547e1 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -25,6 +25,7 @@
 #include <linux/freezer.h>
 #include <linux/stddef.h>
 #include <linux/uaccess.h>
+#include <linux/sizes.h>
 #include <linux/string.h>
 #include <linux/tracehook.h>
 #include <linux/ratelimit.h>
@@ -56,18 +57,27 @@ struct rt_sigframe_user_layout {
 
 	unsigned long fpsimd_offset;
 	unsigned long esr_offset;
+	unsigned long extra_offset;
 	unsigned long end_offset;
 };
 
+#define BASE_SIGFRAME_SIZE round_up(sizeof(struct rt_sigframe), 16)
+#define TERMINATOR_SIZE round_up(sizeof(struct _aarch64_ctx), 16)
+#define EXTRA_CONTEXT_SIZE round_up(sizeof(struct extra_context), 16)
+
 static void init_user_layout(struct rt_sigframe_user_layout *user)
 {
+	const size_t reserved_size =
+		sizeof(user->sigframe->uc.uc_mcontext.__reserved);
+
 	memset(user, 0, sizeof(*user));
 	user->size = offsetof(struct rt_sigframe, uc.uc_mcontext.__reserved);
 
-	user->limit = user->size +
-		sizeof(user->sigframe->uc.uc_mcontext.__reserved) -
-		round_up(sizeof(struct _aarch64_ctx), 16);
-		/* ^ reserve space for terminator */
+	user->limit = user->size + reserved_size;
+
+	user->limit -= TERMINATOR_SIZE;
+	user->limit -= EXTRA_CONTEXT_SIZE;
+	/* Reserve space for extension and terminator ^ */
 }
 
 static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
@@ -75,6 +85,48 @@ static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
 	return round_up(max(user->size, sizeof(struct rt_sigframe)), 16);
 }
 
+/* Sanity limit on the maximum size of signal frame we'll try to generate. */
+/* This is NOT ABI. */
+#define SIGFRAME_MAXSZ SZ_64K
+
+static int __sigframe_alloc(struct rt_sigframe_user_layout *user,
+			    unsigned long *offset, size_t size, bool extend)
+{
+	size_t padded_size = round_up(size, 16);
+
+	if (padded_size > user->limit - user->size &&
+	    !user->extra_offset &&
+	    extend) {
+		int ret;
+
+		ret = __sigframe_alloc(user, &user->extra_offset,
+				       sizeof(struct extra_context), false);
+		if (ret)
+			return ret;
+
+		/*
+		 * Further allocations must go after the fixed-size
+		 * part of the signal frame:
+		 */
+		user->size = BASE_SIGFRAME_SIZE;
+
+		/*
+		 * Allow expansion up to SIGFRAME_MAXSZ, ensuring space for
+		 * the terminator:
+		 */
+		user->limit = SIGFRAME_MAXSZ - TERMINATOR_SIZE;
+	}
+
+	/* Still not enough space?  Bad luck! */
+	if (padded_size > user->limit - user->size)
+		return -ENOMEM;
+
+	*offset = user->size;
+	user->size += padded_size;
+
+	return 0;
+}
+
 /*
  * Allocate space for an optional record of <size> bytes in the user
  * signal frame.  The offset from the signal frame base address to the
@@ -83,11 +135,24 @@ static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
 static int sigframe_alloc(struct rt_sigframe_user_layout *user,
 			  unsigned long *offset, size_t size)
 {
-	size_t padded_size = round_up(size, 16);
+	return __sigframe_alloc(user, offset, size, true);
+}
 
-	*offset = user->size;
-	user->size += padded_size;
+/* Allocate the null terminator record and prevent further allocations */
+static int sigframe_alloc_end(struct rt_sigframe_user_layout *user)
+{
+	int ret;
+
+	/* Un-reserve the space reserved for the terminator: */
+	user->limit += TERMINATOR_SIZE;
+
+	ret = sigframe_alloc(user, &user->end_offset,
+			     sizeof(struct _aarch64_ctx));
+	if (ret)
+		return ret;
 
+	/* Prevent further allocation: */
+	user->limit = user->size;
 	return 0;
 }
 
@@ -314,17 +379,7 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
 			return err;
 	}
 
-	/*
-	 * Allocate space for the terminator record.
-	 * HACK: here we undo the reservation of space for the end record.
-	 * This bodge should be replaced with a cleaner approach later on.
-	 */
-	user->limit = offsetof(struct rt_sigframe, uc.uc_mcontext.__reserved) +
-		sizeof(user->sigframe->uc.uc_mcontext.__reserved);
-
-	err = sigframe_alloc(user, &user->end_offset,
-			     sizeof(struct _aarch64_ctx));
-	return err;
+	return sigframe_alloc_end(user);
 }
 
 
@@ -365,6 +420,26 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
 	}
 
+	if (err == 0 && user->extra_offset) {
+		struct extra_context __user *extra =
+			apply_user_offset(user, user->extra_offset);
+		struct _aarch64_ctx __user *end =
+			(struct _aarch64_ctx __user *)((char __user *)extra +
+				round_up(sizeof(*extra), 16));
+		void __user *extra_data =
+			apply_user_offset(user, BASE_SIGFRAME_SIZE);
+		u32 extra_size = round_up(user->size, 16) - BASE_SIGFRAME_SIZE;
+
+		__put_user_error(EXTRA_MAGIC, &extra->head.magic, err);
+		__put_user_error(sizeof(*extra), &extra->head.size, err);
+		__put_user_error(extra_data, &extra->data, err);
+		__put_user_error(extra_size, &extra->size, err);
+
+		/* Add the terminator */
+		__put_user_error(0, &end->magic, err);
+		__put_user_error(0, &end->size, err);
+	}
+
 	/* set the "end" magic */
 	if (err == 0) {
 		struct _aarch64_ctx __user *end =
-- 
2.1.4

[RFC PATCH v2 5/6] arm64: signal: Parse extra_context during sigreturn

[Dave Martin]

If extra_context is present, parse it.

To avoid abuse by userspace, this patch attempts to ensure that:
 * no more than one extra_context is accepted;
 * the extra_context is a sensible size;
 * the extra context data is properly aligned.

The extra_context data is required to start immediately after
struct rt_sigframe (as during signal delivery).  This serves as a
sanity-check that the signal frame has not been moved or copied
without taking the extra data into account.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/include/uapi/asm/sigcontext.h |  6 ++++-
 arch/arm64/kernel/signal.c               | 46 ++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index b5e2523..a2f7211 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h
@@ -96,7 +96,11 @@ struct esr_context {
  * extra_context must be the last record in sigcontext.__reserved[]
  * except for the terminator).
  *
- * 4) The extra space must itself be terminated with a null
+ * 4) The extra space to which data points must start at the first
+ * 16-byte aligned address immediately after the end of the sigcontext
+ * strucutre.
+ *
+ * 5) The extra space must itself be terminated with a null
  * _aarch64_ctx.
  */
 #define EXTRA_MAGIC	0x45585401
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 95547e1..983cddf 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -223,6 +223,10 @@ static int parse_user_sigframe(struct user_ctxs *user,
 	char __user *base = (char __user *)&sc->__reserved;
 	size_t offset = 0;
 	size_t limit = sizeof(sc->__reserved);
+	bool have_extra_context = false;
+
+	/* Expected location of extra_data (if present): */
+	char __user *const extra_base = (char __user *)sf + BASE_SIGFRAME_SIZE;
 
 	user->fpsimd = NULL;
 
@@ -232,6 +236,9 @@ static int parse_user_sigframe(struct user_ctxs *user,
 	while (1) {
 		int err = 0;
 		u32 magic, size;
+		struct extra_context const __user *extra;
+		void __user *extra_data;
+		u32 extra_size;
 
 		if (limit - offset < sizeof(*head))
 			goto invalid;
@@ -269,6 +276,45 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			/* ignore */
 			break;
 
+		case EXTRA_MAGIC:
+			if (have_extra_context)
+				goto invalid;
+
+			if (size < sizeof(*extra))
+				goto invalid;
+
+			extra = (struct extra_context const __user *)head;
+			__get_user_error(extra_data, &extra->data, err);
+			__get_user_error(extra_size, &extra->size, err);
+			if (err)
+				return err;
+
+			/* Prevent looping/repeated parsing of extra_conext */
+			have_extra_context = true;
+
+			/*
+			 * Rely on the __user accessors to reject bogus
+			 * pointers.
+			 */
+			base = extra_data;
+			if (!IS_ALIGNED((unsigned long)base, 16))
+				goto invalid;
+
+			if (extra_data != extra_base)
+				goto invalid;
+
+			/* Reject "unreasonably large" frames: */
+			limit = extra_size;
+			if (limit > SIGFRAME_MAXSZ - sizeof(sc->__reserved))
+				goto invalid;
+
+			/*
+			 * Ignore trailing terminator in __reserved[]
+			 * and start parsing extra_data:
+			 */
+			offset = 0;
+			continue;
+
 		default:
 			goto invalid;
 		}
-- 
2.1.4

[RFC PATCH v2 5/6] arm64: signal: Parse extra_context during sigreturn

[Dave Martin]

If extra_context is present, parse it.

To avoid abuse by userspace, this patch attempts to ensure that:
 * no more than one extra_context is accepted;
 * the extra_context is a sensible size;
 * the extra context data is properly aligned.

The extra_context data is required to start immediately after
struct rt_sigframe (as during signal delivery).  This serves as a
sanity-check that the signal frame has not been moved or copied
without taking the extra data into account.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/include/uapi/asm/sigcontext.h |  6 ++++-
 arch/arm64/kernel/signal.c               | 46 ++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index b5e2523..a2f7211 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h
@@ -96,7 +96,11 @@ struct esr_context {
  * extra_context must be the last record in sigcontext.__reserved[]
  * except for the terminator).
  *
- * 4) The extra space must itself be terminated with a null
+ * 4) The extra space to which data points must start at the first
+ * 16-byte aligned address immediately after the end of the sigcontext
+ * strucutre.
+ *
+ * 5) The extra space must itself be terminated with a null
  * _aarch64_ctx.
  */
 #define EXTRA_MAGIC	0x45585401
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 95547e1..983cddf 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -223,6 +223,10 @@ static int parse_user_sigframe(struct user_ctxs *user,
 	char __user *base = (char __user *)&sc->__reserved;
 	size_t offset = 0;
 	size_t limit = sizeof(sc->__reserved);
+	bool have_extra_context = false;
+
+	/* Expected location of extra_data (if present): */
+	char __user *const extra_base = (char __user *)sf + BASE_SIGFRAME_SIZE;
 
 	user->fpsimd = NULL;
 
@@ -232,6 +236,9 @@ static int parse_user_sigframe(struct user_ctxs *user,
 	while (1) {
 		int err = 0;
 		u32 magic, size;
+		struct extra_context const __user *extra;
+		void __user *extra_data;
+		u32 extra_size;
 
 		if (limit - offset < sizeof(*head))
 			goto invalid;
@@ -269,6 +276,45 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			/* ignore */
 			break;
 
+		case EXTRA_MAGIC:
+			if (have_extra_context)
+				goto invalid;
+
+			if (size < sizeof(*extra))
+				goto invalid;
+
+			extra = (struct extra_context const __user *)head;
+			__get_user_error(extra_data, &extra->data, err);
+			__get_user_error(extra_size, &extra->size, err);
+			if (err)
+				return err;
+
+			/* Prevent looping/repeated parsing of extra_conext */
+			have_extra_context = true;
+
+			/*
+			 * Rely on the __user accessors to reject bogus
+			 * pointers.
+			 */
+			base = extra_data;
+			if (!IS_ALIGNED((unsigned long)base, 16))
+				goto invalid;
+
+			if (extra_data != extra_base)
+				goto invalid;
+
+			/* Reject "unreasonably large" frames: */
+			limit = extra_size;
+			if (limit > SIGFRAME_MAXSZ - sizeof(sc->__reserved))
+				goto invalid;
+
+			/*
+			 * Ignore trailing terminator in __reserved[]
+			 * and start parsing extra_data:
+			 */
+			offset = 0;
+			continue;
+
 		default:
 			goto invalid;
 		}
-- 
2.1.4

[RFC PATCH v2 6/6] arm64: signal: Report signal frame size to userspace via auxv

[Dave Martin]

Stateful CPU architecture extensions may require the signal frame
to grow to a size that exceeds the arch's MINSIGSTKSZ #define.
However, changing this #define is an ABI break.

To allow userspace the option of determining the signal frame size
in a more forwards-compatible way, this patch adds a new auxv entry
tagged with AT_MINSIGSTKSZ, which provides the maximum signal frame
size that the process can observe during its lifetime.

If AT_MINSIGSTKSZ is absent from the aux vector, the caller can
assume that the MINSIGSTKSZ #define is sufficient.  This allows for
a consistent interface with older kernels that do not provide
AT_MINSIGSTKSZ.

The idea is that libc could expose this via sysconf() or some
similar mechanism.

There is deliberately no AT_SIGSTKSZ.  The kernel knows nothing
about userspace's own stack overheads and should not pretend to
know.

For arm64:

The primary motivation for this interface is the Scalable Vector
Extension, which can require at least 4KB or so of extra space
in the signal frame for the largest hardware implementations.

To determine the correct value, a "Christmas tree" mode (via the
add_all argument) is added to setup_sigframe_layout(), to simulate
addition of all possible records to the signal frame at maximum
possible size.

If this procedure goes wrong somehow, resulting in a stupidly large
frame layout and hence failure of sigframe_alloc() to allocate a
record to the frame, then this is indicative of a kernel bug: the
kernel's internal SIGFRAME_MAXSZ is supposed to sanity-check
against generting frames that we consider _impossibly_ large.  In
this case, SIGSTKSZ is returned as a "reasonable guess that is at
least bigger than MINSIGSTKSZ" and we WARN().

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---

Notes:

1)

Should AT_MINSIGSTKSZ be defined globally?

If there's a likelihood that one or more other arches may make use
of this mechanism, then the define could be moved to
linux/include/uapi/linux/auxvec.h.

Either way, userspace code can do #ifdef AT_MINSIGSTKSZ, so the
define can be left arch-specific without serious source
incompatibility for userspace.  The chosen number is ABI though:
currently it's in the arch-specific space (>= 32).

2)

The kernel has an ABI commitment not to generate larger signal
frames than userspace expects, so another mechanism would be needed
to "turn on" the generation of larger frames.

For now I don't anticipate a global mechanism for this.  For SVE,
I currently have a mechanism to limit the vector length given to
new processes by default, thus avoiding signal frames larger than
MINSIGSTKSZ being seen unless the program explicitly asks for
longer vectors.

3)

This patch feels like an abuse of ARCH_DLINFO, but I didn't
feel it was worth hacking the core code just for this...

If anyone objects, I'm happy to propose a new macro (or a new name
for the existing macro).

4)

For arm64, when the signal frame has not been enlarged, the true
size is returned, which is slightly smaller than MINSIGSTKSZ.

Since callers must know their own stack overheads in order to make
use of the result, and since we have no way to know those, this
probably doesn't matter.

There could be an argument for returning MINSIGSTKSZ though, in
case there is software that assumes for other reasons that stacks
are at least MINSIGSTKSZ in size.  By definition this interface
can't expect to ensure 100% compatibility, so it's not clear
whether this matters.  Userspace that cares could equally implement
this clamping itself.

---
 arch/arm64/include/asm/elf.h         |  5 +++++
 arch/arm64/include/asm/processor.h   |  3 +++
 arch/arm64/include/uapi/asm/auxvec.h |  3 ++-
 arch/arm64/kernel/signal.c           | 36 +++++++++++++++++++++++++++++++-----
 4 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 5d17004..5958487 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -24,6 +24,10 @@
 #include <asm/ptrace.h>
 #include <asm/user.h>
 
+#ifndef __ASSEMBLY__
+#include <asm/processor.h> /* for get_minsigstksz(), used by ARCH_DLINFO */
+#endif
+
 /*
  * AArch64 static relocation types.
  */
@@ -149,6 +153,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
 do {									\
 	NEW_AUX_ENT(AT_SYSINFO_EHDR,					\
 		    (elf_addr_t)current->mm->context.vdso);		\
+	NEW_AUX_ENT(AT_MINSIGSTKSZ, get_minsigstksz());			\
 } while (0)
 
 #define ARCH_HAS_SETUP_ADDITIONAL_PAGES
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 0502007..9bf5804 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -194,4 +194,7 @@ static inline void spin_lock_prefetch(const void *ptr)
 int cpu_enable_pan(void *__unused);
 int cpu_enable_cache_maint_trap(void *__unused);
 
+/* User signal frame size discovery: */
+int get_minsigstksz(void);
+
 #endif /* __ASM_PROCESSOR_H */
diff --git a/arch/arm64/include/uapi/asm/auxvec.h b/arch/arm64/include/uapi/asm/auxvec.h
index 4cf0c17..1d45b28 100644
--- a/arch/arm64/include/uapi/asm/auxvec.h
+++ b/arch/arm64/include/uapi/asm/auxvec.h
@@ -18,7 +18,8 @@
 
 /* vDSO location */
 #define AT_SYSINFO_EHDR	33
+#define AT_MINSIGSTKSZ	34	/* stack needed for signal delivery */
 
-#define AT_VECTOR_SIZE_ARCH 1 /* entries in ARCH_DLINFO */
+#define AT_VECTOR_SIZE_ARCH 2 /* entries in ARCH_DLINFO */
 
 #endif
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 983cddf..c4ac046 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -407,8 +407,15 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 	return 0;
 }
 
-/* Determine the layout of optional records in the signal frame */
-static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
+/*
+ * Determine the layout of optional records in the signal frame
+ *
+ * add_all: if true, lays out the biggest possible signal frame for
+ *	this task; otherwise, generates a layout for the current state
+ *	of the task.
+ */
+static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
+				 bool add_all)
 {
 	int err;
 
@@ -418,7 +425,7 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
 		return err;
 
 	/* fault information, if valid */
-	if (current->thread.fault_code) {
+	if (add_all || current->thread.fault_code) {
 		err = sigframe_alloc(user, &user->esr_offset,
 				     sizeof(struct esr_context));
 		if (err)
@@ -428,7 +435,6 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
 	return sigframe_alloc_end(user);
 }
 
-
 static int setup_sigframe(struct rt_sigframe_user_layout *user,
 			  struct pt_regs *regs, sigset_t *set)
 {
@@ -505,7 +511,7 @@ static int get_sigframe(struct rt_sigframe_user_layout *user,
 	int err;
 
 	init_user_layout(user);
-	err = setup_sigframe_layout(user);
+	err = setup_sigframe_layout(user, false);
 	if (err)
 		return err;
 
@@ -728,3 +734,23 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
 		thread_flags = READ_ONCE(current_thread_info()->flags);
 	} while (thread_flags & _TIF_WORK_MASK);
 }
+
+/*
+ * Determine the stack space required for guaranteed signal devliery.
+ * This function is used to populate AT_MINSIGSTKSZ at process startup.
+ */
+int get_minsigstksz(void)
+{
+	struct rt_sigframe_user_layout user;
+	int err;
+
+	init_user_layout(&user);
+	err = setup_sigframe_layout(&user, true);
+
+	if (err) {
+		WARN_ON(1);
+
+		return SIGSTKSZ;
+	} else
+		return sigframe_size(&user) + 16; /* max alignment padding */
+}
-- 
2.1.4

[RFC PATCH v2 6/6] arm64: signal: Report signal frame size to userspace via auxv

[Dave Martin]

Stateful CPU architecture extensions may require the signal frame
to grow to a size that exceeds the arch's MINSIGSTKSZ #define.
However, changing this #define is an ABI break.

To allow userspace the option of determining the signal frame size
in a more forwards-compatible way, this patch adds a new auxv entry
tagged with AT_MINSIGSTKSZ, which provides the maximum signal frame
size that the process can observe during its lifetime.

If AT_MINSIGSTKSZ is absent from the aux vector, the caller can
assume that the MINSIGSTKSZ #define is sufficient.  This allows for
a consistent interface with older kernels that do not provide
AT_MINSIGSTKSZ.

The idea is that libc could expose this via sysconf() or some
similar mechanism.

There is deliberately no AT_SIGSTKSZ.  The kernel knows nothing
about userspace's own stack overheads and should not pretend to
know.

For arm64:

The primary motivation for this interface is the Scalable Vector
Extension, which can require at least 4KB or so of extra space
in the signal frame for the largest hardware implementations.

To determine the correct value, a "Christmas tree" mode (via the
add_all argument) is added to setup_sigframe_layout(), to simulate
addition of all possible records to the signal frame at maximum
possible size.

If this procedure goes wrong somehow, resulting in a stupidly large
frame layout and hence failure of sigframe_alloc() to allocate a
record to the frame, then this is indicative of a kernel bug: the
kernel's internal SIGFRAME_MAXSZ is supposed to sanity-check
against generting frames that we consider _impossibly_ large.  In
this case, SIGSTKSZ is returned as a "reasonable guess that is at
least bigger than MINSIGSTKSZ" and we WARN().

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---

Notes:

1)

Should AT_MINSIGSTKSZ be defined globally?

If there's a likelihood that one or more other arches may make use
of this mechanism, then the define could be moved to
linux/include/uapi/linux/auxvec.h.

Either way, userspace code can do #ifdef AT_MINSIGSTKSZ, so the
define can be left arch-specific without serious source
incompatibility for userspace.  The chosen number is ABI though:
currently it's in the arch-specific space (>= 32).

2)

The kernel has an ABI commitment not to generate larger signal
frames than userspace expects, so another mechanism would be needed
to "turn on" the generation of larger frames.

For now I don't anticipate a global mechanism for this.  For SVE,
I currently have a mechanism to limit the vector length given to
new processes by default, thus avoiding signal frames larger than
MINSIGSTKSZ being seen unless the program explicitly asks for
longer vectors.

3)

This patch feels like an abuse of ARCH_DLINFO, but I didn't
feel it was worth hacking the core code just for this...

If anyone objects, I'm happy to propose a new macro (or a new name
for the existing macro).

4)

For arm64, when the signal frame has not been enlarged, the true
size is returned, which is slightly smaller than MINSIGSTKSZ.

Since callers must know their own stack overheads in order to make
use of the result, and since we have no way to know those, this
probably doesn't matter.

There could be an argument for returning MINSIGSTKSZ though, in
case there is software that assumes for other reasons that stacks
are at least MINSIGSTKSZ in size.  By definition this interface
can't expect to ensure 100% compatibility, so it's not clear
whether this matters.  Userspace that cares could equally implement
this clamping itself.

---
 arch/arm64/include/asm/elf.h         |  5 +++++
 arch/arm64/include/asm/processor.h   |  3 +++
 arch/arm64/include/uapi/asm/auxvec.h |  3 ++-
 arch/arm64/kernel/signal.c           | 36 +++++++++++++++++++++++++++++++-----
 4 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 5d17004..5958487 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -24,6 +24,10 @@
 #include <asm/ptrace.h>
 #include <asm/user.h>
 
+#ifndef __ASSEMBLY__
+#include <asm/processor.h> /* for get_minsigstksz(), used by ARCH_DLINFO */
+#endif
+
 /*
  * AArch64 static relocation types.
  */
@@ -149,6 +153,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
 do {									\
 	NEW_AUX_ENT(AT_SYSINFO_EHDR,					\
 		    (elf_addr_t)current->mm->context.vdso);		\
+	NEW_AUX_ENT(AT_MINSIGSTKSZ, get_minsigstksz());			\
 } while (0)
 
 #define ARCH_HAS_SETUP_ADDITIONAL_PAGES
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 0502007..9bf5804 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -194,4 +194,7 @@ static inline void spin_lock_prefetch(const void *ptr)
 int cpu_enable_pan(void *__unused);
 int cpu_enable_cache_maint_trap(void *__unused);
 
+/* User signal frame size discovery: */
+int get_minsigstksz(void);
+
 #endif /* __ASM_PROCESSOR_H */
diff --git a/arch/arm64/include/uapi/asm/auxvec.h b/arch/arm64/include/uapi/asm/auxvec.h
index 4cf0c17..1d45b28 100644
--- a/arch/arm64/include/uapi/asm/auxvec.h
+++ b/arch/arm64/include/uapi/asm/auxvec.h
@@ -18,7 +18,8 @@
 
 /* vDSO location */
 #define AT_SYSINFO_EHDR	33
+#define AT_MINSIGSTKSZ	34	/* stack needed for signal delivery */
 
-#define AT_VECTOR_SIZE_ARCH 1 /* entries in ARCH_DLINFO */
+#define AT_VECTOR_SIZE_ARCH 2 /* entries in ARCH_DLINFO */
 
 #endif
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 983cddf..c4ac046 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -407,8 +407,15 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 	return 0;
 }
 
-/* Determine the layout of optional records in the signal frame */
-static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
+/*
+ * Determine the layout of optional records in the signal frame
+ *
+ * add_all: if true, lays out the biggest possible signal frame for
+ *	this task; otherwise, generates a layout for the current state
+ *	of the task.
+ */
+static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
+				 bool add_all)
 {
 	int err;
 
@@ -418,7 +425,7 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
 		return err;
 
 	/* fault information, if valid */
-	if (current->thread.fault_code) {
+	if (add_all || current->thread.fault_code) {
 		err = sigframe_alloc(user, &user->esr_offset,
 				     sizeof(struct esr_context));
 		if (err)
@@ -428,7 +435,6 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
 	return sigframe_alloc_end(user);
 }
 
-
 static int setup_sigframe(struct rt_sigframe_user_layout *user,
 			  struct pt_regs *regs, sigset_t *set)
 {
@@ -505,7 +511,7 @@ static int get_sigframe(struct rt_sigframe_user_layout *user,
 	int err;
 
 	init_user_layout(user);
-	err = setup_sigframe_layout(user);
+	err = setup_sigframe_layout(user, false);
 	if (err)
 		return err;
 
@@ -728,3 +734,23 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
 		thread_flags = READ_ONCE(current_thread_info()->flags);
 	} while (thread_flags & _TIF_WORK_MASK);
 }
+
+/*
+ * Determine the stack space required for guaranteed signal devliery.
+ * This function is used to populate AT_MINSIGSTKSZ at process startup.
+ */
+int get_minsigstksz(void)
+{
+	struct rt_sigframe_user_layout user;
+	int err;
+
+	init_user_layout(&user);
+	err = setup_sigframe_layout(&user, true);
+
+	if (err) {
+		WARN_ON(1);
+
+		return SIGSTKSZ;
+	} else
+		return sigframe_size(&user) + 16; /* max alignment padding */
+}
-- 
2.1.4

Re: [RFC PATCH v2 0/6] Signal frame expansion support

[Michael Ellerman]

Dave Martin <Dave.Martin@arm.com> writes:

> (Note: This is an arm64-specific series, but the concepts introduced may
> be of interest to other arches -- see in particular patch 6.)

Hi Dave,

I think we (powerpc) will probably want to use this too.

I haven't dug into the details yet, I'll try and do that after the 4.12
merge window.

cheers

[RFC PATCH v2 0/6] Signal frame expansion support

[Michael Ellerman]

Dave Martin <Dave.Martin@arm.com> writes:

> (Note: This is an arm64-specific series, but the concepts introduced may
> be of interest to other arches -- see in particular patch 6.)

Hi Dave,

I think we (powerpc) will probably want to use this too.

I haven't dug into the details yet, I'll try and do that after the 4.12
merge window.

cheers

Re: [RFC PATCH v2 0/6] Signal frame expansion support

[Dave Martin]

On Thu, Apr 20, 2017 at 09:49:36PM +1000, Michael Ellerman wrote:
> Dave Martin <Dave.Martin@arm.com> writes:
> 
> > (Note: This is an arm64-specific series, but the concepts introduced may
> > be of interest to other arches -- see in particular patch 6.)
> 
> Hi Dave,
> 
> I think we (powerpc) will probably want to use this too.
> 
> I haven't dug into the details yet, I'll try and do that after the 4.12
> merge window.

That would be great, thanks.

Since I've been focusing on arm64 and SVE, there may be generalities
or design issues that I've missed.

Cheers
---Dave

[RFC PATCH v2 0/6] Signal frame expansion support

[Dave Martin]

On Thu, Apr 20, 2017 at 09:49:36PM +1000, Michael Ellerman wrote:
> Dave Martin <Dave.Martin@arm.com> writes:
> 
> > (Note: This is an arm64-specific series, but the concepts introduced may
> > be of interest to other arches -- see in particular patch 6.)
> 
> Hi Dave,
> 
> I think we (powerpc) will probably want to use this too.
> 
> I haven't dug into the details yet, I'll try and do that after the 4.12
> merge window.

That would be great, thanks.

Since I've been focusing on arm64 and SVE, there may be generalities
or design issues that I've missed.

Cheers
---Dave

Re: [RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Catalin Marinas]

Hi Dave,

On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:
> --- a/arch/arm64/include/uapi/asm/sigcontext.h
> +++ b/arch/arm64/include/uapi/asm/sigcontext.h
> @@ -80,4 +80,31 @@ struct esr_context {
>  	__u64 esr;
>  };
>  
> +/*
> + * Pointer to extra space for additional structures that don't fit in
> + * sigcontext.__reserved[].  Note:
> + *
> + * 1) fpsimd_context, esr_context and extra_context must be placed in
> + * sigcontext.__reserved[] if present.  They cannot be placed in the
> + * extra space.  Any other record can be placed either in the extra
> + * space or in sigcontext.__reserved[].
> + *
> + * 2) There must not be more than one extra_context.
> + *
> + * 3) If extra_context is present, it must be followed immediately in
> + * sigcontext.__reserved[] by the terminating null _aarch64_ctx (i.e.,
> + * extra_context must be the last record in sigcontext.__reserved[]
> + * except for the terminator).
> + *
> + * 4) The extra space must itself be terminated with a null
> + * _aarch64_ctx.
> + */

IIUC, if we need to save some state that doesn't fit in what's left of
sigcontext.__reserved[] (e.g. SVE with 1024-bit vector length), we
ignore the available space and go for a memory block following the end
of sigcontext.__reserved[] + 16. Is there a reason we can't store the
new state across the end of sigcontext.__reserved[] and move fp/lr at
the end of the new frame? I'm not sure the fp/lr position immediately
after __reserved[] counts as ABI.

> +#define EXTRA_MAGIC	0x45585401
> +
> +struct extra_context {
> +	struct _aarch64_ctx head;
> +	void __user *data;	/* 16-byte aligned pointer to extra space */
"__user" is a kernel-only attribute, we shouldn't expose it in a uapi
header.

> +	__u32 size;		/* size in bytes of the extra space */
> +};

Do we need the size of the extra space? Can we not infer it anyway by
walking the contexts save there? Surely we don't expect more than one
extra context.

-- 
Catalin

[RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Catalin Marinas]

Hi Dave,

On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:
> --- a/arch/arm64/include/uapi/asm/sigcontext.h
> +++ b/arch/arm64/include/uapi/asm/sigcontext.h
> @@ -80,4 +80,31 @@ struct esr_context {
>  	__u64 esr;
>  };
>  
> +/*
> + * Pointer to extra space for additional structures that don't fit in
> + * sigcontext.__reserved[].  Note:
> + *
> + * 1) fpsimd_context, esr_context and extra_context must be placed in
> + * sigcontext.__reserved[] if present.  They cannot be placed in the
> + * extra space.  Any other record can be placed either in the extra
> + * space or in sigcontext.__reserved[].
> + *
> + * 2) There must not be more than one extra_context.
> + *
> + * 3) If extra_context is present, it must be followed immediately in
> + * sigcontext.__reserved[] by the terminating null _aarch64_ctx (i.e.,
> + * extra_context must be the last record in sigcontext.__reserved[]
> + * except for the terminator).
> + *
> + * 4) The extra space must itself be terminated with a null
> + * _aarch64_ctx.
> + */

IIUC, if we need to save some state that doesn't fit in what's left of
sigcontext.__reserved[] (e.g. SVE with 1024-bit vector length), we
ignore the available space and go for a memory block following the end
of sigcontext.__reserved[] + 16. Is there a reason we can't store the
new state across the end of sigcontext.__reserved[] and move fp/lr at
the end of the new frame? I'm not sure the fp/lr position immediately
after __reserved[] counts as ABI.

> +#define EXTRA_MAGIC	0x45585401
> +
> +struct extra_context {
> +	struct _aarch64_ctx head;
> +	void __user *data;	/* 16-byte aligned pointer to extra space */
"__user" is a kernel-only attribute, we shouldn't expose it in a uapi
header.

> +	__u32 size;		/* size in bytes of the extra space */
> +};

Do we need the size of the extra space? Can we not infer it anyway by
walking the contexts save there? Surely we don't expect more than one
extra context.

-- 
Catalin

Re: [RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Dave Martin]

On Fri, May 12, 2017 at 05:57:24PM +0100, Catalin Marinas wrote:
> Hi Dave,
> 
> On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:
> > --- a/arch/arm64/include/uapi/asm/sigcontext.h
> > +++ b/arch/arm64/include/uapi/asm/sigcontext.h
> > @@ -80,4 +80,31 @@ struct esr_context {
> >  	__u64 esr;
> >  };
> >  
> > +/*
> > + * Pointer to extra space for additional structures that don't fit in
> > + * sigcontext.__reserved[].  Note:
> > + *
> > + * 1) fpsimd_context, esr_context and extra_context must be placed in
> > + * sigcontext.__reserved[] if present.  They cannot be placed in the
> > + * extra space.  Any other record can be placed either in the extra
> > + * space or in sigcontext.__reserved[].
> > + *
> > + * 2) There must not be more than one extra_context.
> > + *
> > + * 3) If extra_context is present, it must be followed immediately in
> > + * sigcontext.__reserved[] by the terminating null _aarch64_ctx (i.e.,
> > + * extra_context must be the last record in sigcontext.__reserved[]
> > + * except for the terminator).
> > + *
> > + * 4) The extra space must itself be terminated with a null
> > + * _aarch64_ctx.
> > + */
> 
> IIUC, if we need to save some state that doesn't fit in what's left of
> sigcontext.__reserved[] (e.g. SVE with 1024-bit vector length), we
> ignore the available space and go for a memory block following the end
> of sigcontext.__reserved[] + 16. Is there a reason we can't store the
> new state across the end of sigcontext.__reserved[] and move fp/lr at
> the end of the new frame? I'm not sure the fp/lr position immediately
> after __reserved[] counts as ABI.

This was my original view.

Originally I preferred not to waste the space and did move fp/lr to the
end, but someone (I think you or Will) expressed concern that the fp/lr
position relative to the signal frame _might_ count as ABI.

I think it's not that likely that software will be relying on this,
since it appears easier just to follow the frame chain than to treat
this as a special case.

But it's hard to be certain.  It comes down to a judgement call.

We could also move fp/lr later, if we require sigframe parsers to be
a bit more permissive about where extra_context starts.

> > +#define EXTRA_MAGIC	0x45585401
> > +
> > +struct extra_context {
> > +	struct _aarch64_ctx head;
> > +	void __user *data;	/* 16-byte aligned pointer to extra space */
> "__user" is a kernel-only attribute, we shouldn't expose it in a uapi
> header.

This is filtered out by headers_install, just like #ifdef __KERNEL__.

data really is a pointer to a user address.  Compare for example struct
iovec in include/uapi/linux/uio.h.

I think the __user is required to keep sparse happy, otherwise some
additional casting is needed to bless a valid read from this field
before passing it to get/put_user.  Or there may be some other reason
I've forgotten...

> 
> > +	__u32 size;		/* size in bytes of the extra space */
> > +};
> 
> Do we need the size of the extra space? Can we not infer it anyway by
> walking the contexts save there? Surely we don't expect more than one
> extra context.

Strictly speaking we don't need it.  When userspace parses a signal
frame generated by the kernel, it can trust the kernel to write a well-
formed signal frame.

In sigreturn it allows us to retain a sanity-check on overall size
similar to what sizeof(__reserved) gives us.  This "feels cleaner"
to me, but the value of it is debatable, since we can still apply
SIGFRAME_MAXSZ and uaccess should protect us against gross overruns.

The size parameter *may* provide some limited defence against
sigreturn-mediated stack smashing in userspace, but it wasn't designed
for this purpose and again the value is debatable.

We can likely live without it if you're not keen on it.

Cheers
---Dave

[RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Dave Martin]

On Fri, May 12, 2017 at 05:57:24PM +0100, Catalin Marinas wrote:
> Hi Dave,
> 
> On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:
> > --- a/arch/arm64/include/uapi/asm/sigcontext.h
> > +++ b/arch/arm64/include/uapi/asm/sigcontext.h
> > @@ -80,4 +80,31 @@ struct esr_context {
> >  	__u64 esr;
> >  };
> >  
> > +/*
> > + * Pointer to extra space for additional structures that don't fit in
> > + * sigcontext.__reserved[].  Note:
> > + *
> > + * 1) fpsimd_context, esr_context and extra_context must be placed in
> > + * sigcontext.__reserved[] if present.  They cannot be placed in the
> > + * extra space.  Any other record can be placed either in the extra
> > + * space or in sigcontext.__reserved[].
> > + *
> > + * 2) There must not be more than one extra_context.
> > + *
> > + * 3) If extra_context is present, it must be followed immediately in
> > + * sigcontext.__reserved[] by the terminating null _aarch64_ctx (i.e.,
> > + * extra_context must be the last record in sigcontext.__reserved[]
> > + * except for the terminator).
> > + *
> > + * 4) The extra space must itself be terminated with a null
> > + * _aarch64_ctx.
> > + */
> 
> IIUC, if we need to save some state that doesn't fit in what's left of
> sigcontext.__reserved[] (e.g. SVE with 1024-bit vector length), we
> ignore the available space and go for a memory block following the end
> of sigcontext.__reserved[] + 16. Is there a reason we can't store the
> new state across the end of sigcontext.__reserved[] and move fp/lr at
> the end of the new frame? I'm not sure the fp/lr position immediately
> after __reserved[] counts as ABI.

This was my original view.

Originally I preferred not to waste the space and did move fp/lr to the
end, but someone (I think you or Will) expressed concern that the fp/lr
position relative to the signal frame _might_ count as ABI.

I think it's not that likely that software will be relying on this,
since it appears easier just to follow the frame chain than to treat
this as a special case.

But it's hard to be certain.  It comes down to a judgement call.

We could also move fp/lr later, if we require sigframe parsers to be
a bit more permissive about where extra_context starts.

> > +#define EXTRA_MAGIC	0x45585401
> > +
> > +struct extra_context {
> > +	struct _aarch64_ctx head;
> > +	void __user *data;	/* 16-byte aligned pointer to extra space */
> "__user" is a kernel-only attribute, we shouldn't expose it in a uapi
> header.

This is filtered out by headers_install, just like #ifdef __KERNEL__.

data really is a pointer to a user address.  Compare for example struct
iovec in include/uapi/linux/uio.h.

I think the __user is required to keep sparse happy, otherwise some
additional casting is needed to bless a valid read from this field
before passing it to get/put_user.  Or there may be some other reason
I've forgotten...

> 
> > +	__u32 size;		/* size in bytes of the extra space */
> > +};
> 
> Do we need the size of the extra space? Can we not infer it anyway by
> walking the contexts save there? Surely we don't expect more than one
> extra context.

Strictly speaking we don't need it.  When userspace parses a signal
frame generated by the kernel, it can trust the kernel to write a well-
formed signal frame.

In sigreturn it allows us to retain a sanity-check on overall size
similar to what sizeof(__reserved) gives us.  This "feels cleaner"
to me, but the value of it is debatable, since we can still apply
SIGFRAME_MAXSZ and uaccess should protect us against gross overruns.

The size parameter *may* provide some limited defence against
sigreturn-mediated stack smashing in userspace, but it wasn't designed
for this purpose and again the value is debatable.

We can likely live without it if you're not keen on it.

Cheers
---Dave

Re: [RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Catalin Marinas]

On Mon, May 15, 2017 at 02:24:45PM +0100, Dave P Martin wrote:
> On Fri, May 12, 2017 at 05:57:24PM +0100, Catalin Marinas wrote:
> > On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:
> > > --- a/arch/arm64/include/uapi/asm/sigcontext.h
> > > +++ b/arch/arm64/include/uapi/asm/sigcontext.h
> > > @@ -80,4 +80,31 @@ struct esr_context {
> > >  	__u64 esr;
> > >  };
> > >  
> > > +/*
> > > + * Pointer to extra space for additional structures that don't fit in
> > > + * sigcontext.__reserved[].  Note:
> > > + *
> > > + * 1) fpsimd_context, esr_context and extra_context must be placed in
> > > + * sigcontext.__reserved[] if present.  They cannot be placed in the
> > > + * extra space.  Any other record can be placed either in the extra
> > > + * space or in sigcontext.__reserved[].
> > > + *
> > > + * 2) There must not be more than one extra_context.
> > > + *
> > > + * 3) If extra_context is present, it must be followed immediately in
> > > + * sigcontext.__reserved[] by the terminating null _aarch64_ctx (i.e.,
> > > + * extra_context must be the last record in sigcontext.__reserved[]
> > > + * except for the terminator).
> > > + *
> > > + * 4) The extra space must itself be terminated with a null
> > > + * _aarch64_ctx.
> > > + */
> > 
> > IIUC, if we need to save some state that doesn't fit in what's left of
> > sigcontext.__reserved[] (e.g. SVE with 1024-bit vector length), we
> > ignore the available space and go for a memory block following the end
> > of sigcontext.__reserved[] + 16. Is there a reason we can't store the
> > new state across the end of sigcontext.__reserved[] and move fp/lr at
> > the end of the new frame? I'm not sure the fp/lr position immediately
> > after __reserved[] counts as ABI.
> 
> This was my original view.
> 
> Originally I preferred not to waste the space and did move fp/lr to the
> end, but someone (I think you or Will) expressed concern that the fp/lr
> position relative to the signal frame _might_ count as ABI.
> 
> I think it's not that likely that software will be relying on this,
> since it appears easier just to follow the frame chain than to treat
> this as a special case.
> 
> But it's hard to be certain.  It comes down to a judgement call.

I would not consider this ABI. The ABI part is that the fp register
points to where fp/lr were saved.

> > > +#define EXTRA_MAGIC	0x45585401
> > > +
> > > +struct extra_context {
> > > +	struct _aarch64_ctx head;
> > > +	void __user *data;	/* 16-byte aligned pointer to extra space */
> > "__user" is a kernel-only attribute, we shouldn't expose it in a uapi
> > header.
> 
> This is filtered out by headers_install, just like #ifdef __KERNEL__.

Ah, ok, I missed this.

> > > +	__u32 size;		/* size in bytes of the extra space */
> > > +};
> > 
> > Do we need the size of the extra space? Can we not infer it anyway by
> > walking the contexts save there? Surely we don't expect more than one
> > extra context.
> 
> Strictly speaking we don't need it.  When userspace parses a signal
> frame generated by the kernel, it can trust the kernel to write a well-
> formed signal frame.
> 
> In sigreturn it allows us to retain a sanity-check on overall size
> similar to what sizeof(__reserved) gives us.  This "feels cleaner"
> to me, but the value of it is debatable, since we can still apply
> SIGFRAME_MAXSZ and uaccess should protect us against gross overruns.

I'm not keen on the size information, it seems superfluous.

BTW, does SIGFRAME_MAXSZ now become ABI? Or the user only needs to
interrogate the frame size and we keep this internal to the kernel?

-- 
Catalin

[RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Catalin Marinas]

On Mon, May 15, 2017 at 02:24:45PM +0100, Dave P Martin wrote:
> On Fri, May 12, 2017 at 05:57:24PM +0100, Catalin Marinas wrote:
> > On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:
> > > --- a/arch/arm64/include/uapi/asm/sigcontext.h
> > > +++ b/arch/arm64/include/uapi/asm/sigcontext.h
> > > @@ -80,4 +80,31 @@ struct esr_context {
> > >  	__u64 esr;
> > >  };
> > >  
> > > +/*
> > > + * Pointer to extra space for additional structures that don't fit in
> > > + * sigcontext.__reserved[].  Note:
> > > + *
> > > + * 1) fpsimd_context, esr_context and extra_context must be placed in
> > > + * sigcontext.__reserved[] if present.  They cannot be placed in the
> > > + * extra space.  Any other record can be placed either in the extra
> > > + * space or in sigcontext.__reserved[].
> > > + *
> > > + * 2) There must not be more than one extra_context.
> > > + *
> > > + * 3) If extra_context is present, it must be followed immediately in
> > > + * sigcontext.__reserved[] by the terminating null _aarch64_ctx (i.e.,
> > > + * extra_context must be the last record in sigcontext.__reserved[]
> > > + * except for the terminator).
> > > + *
> > > + * 4) The extra space must itself be terminated with a null
> > > + * _aarch64_ctx.
> > > + */
> > 
> > IIUC, if we need to save some state that doesn't fit in what's left of
> > sigcontext.__reserved[] (e.g. SVE with 1024-bit vector length), we
> > ignore the available space and go for a memory block following the end
> > of sigcontext.__reserved[] + 16. Is there a reason we can't store the
> > new state across the end of sigcontext.__reserved[] and move fp/lr at
> > the end of the new frame? I'm not sure the fp/lr position immediately
> > after __reserved[] counts as ABI.
> 
> This was my original view.
> 
> Originally I preferred not to waste the space and did move fp/lr to the
> end, but someone (I think you or Will) expressed concern that the fp/lr
> position relative to the signal frame _might_ count as ABI.
> 
> I think it's not that likely that software will be relying on this,
> since it appears easier just to follow the frame chain than to treat
> this as a special case.
> 
> But it's hard to be certain.  It comes down to a judgement call.

I would not consider this ABI. The ABI part is that the fp register
points to where fp/lr were saved.

> > > +#define EXTRA_MAGIC	0x45585401
> > > +
> > > +struct extra_context {
> > > +	struct _aarch64_ctx head;
> > > +	void __user *data;	/* 16-byte aligned pointer to extra space */
> > "__user" is a kernel-only attribute, we shouldn't expose it in a uapi
> > header.
> 
> This is filtered out by headers_install, just like #ifdef __KERNEL__.

Ah, ok, I missed this.

> > > +	__u32 size;		/* size in bytes of the extra space */
> > > +};
> > 
> > Do we need the size of the extra space? Can we not infer it anyway by
> > walking the contexts save there? Surely we don't expect more than one
> > extra context.
> 
> Strictly speaking we don't need it.  When userspace parses a signal
> frame generated by the kernel, it can trust the kernel to write a well-
> formed signal frame.
> 
> In sigreturn it allows us to retain a sanity-check on overall size
> similar to what sizeof(__reserved) gives us.  This "feels cleaner"
> to me, but the value of it is debatable, since we can still apply
> SIGFRAME_MAXSZ and uaccess should protect us against gross overruns.

I'm not keen on the size information, it seems superfluous.

BTW, does SIGFRAME_MAXSZ now become ABI? Or the user only needs to
interrogate the frame size and we keep this internal to the kernel?

-- 
Catalin

Re: [RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Dave Martin]

On Tue, May 23, 2017 at 12:30:19PM +0100, Catalin Marinas wrote:
> On Mon, May 15, 2017 at 02:24:45PM +0100, Dave P Martin wrote:
> > On Fri, May 12, 2017 at 05:57:24PM +0100, Catalin Marinas wrote:
> > > On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:

[...]

> > Originally I preferred not to waste the space and did move fp/lr to the
> > end, but someone (I think you or Will) expressed concern that the fp/lr
> > position relative to the signal frame _might_ count as ABI.
> > 
> > I think it's not that likely that software will be relying on this,
> > since it appears easier just to follow the frame chain than to treat
> > this as a special case.
> > 
> > But it's hard to be certain.  It comes down to a judgement call.
> 
> I would not consider this ABI. The ABI part is that the fp register
> points to where fp/lr were saved.

Fair enough, I'm happy to move it back.  (That was my preferred design
in the first place anyway).

> > > > +#define EXTRA_MAGIC	0x45585401
> > > > +
> > > > +struct extra_context {
> > > > +	struct _aarch64_ctx head;
> > > > +	void __user *data;	/* 16-byte aligned pointer to extra space */
> > > "__user" is a kernel-only attribute, we shouldn't expose it in a uapi
> > > header.
> > 
> > This is filtered out by headers_install, just like #ifdef __KERNEL__.
> 
> Ah, ok, I missed this.

It was a surprise to me too...

> > > > +	__u32 size;		/* size in bytes of the extra space */
> > > > +};
> > > 
> > > Do we need the size of the extra space? Can we not infer it anyway by
> > > walking the contexts save there? Surely we don't expect more than one
> > > extra context.
> > 
> > Strictly speaking we don't need it.  When userspace parses a signal
> > frame generated by the kernel, it can trust the kernel to write a well-
> > formed signal frame.
> > 
> > In sigreturn it allows us to retain a sanity-check on overall size
> > similar to what sizeof(__reserved) gives us.  This "feels cleaner"
> > to me, but the value of it is debatable, since we can still apply
> > SIGFRAME_MAXSZ and uaccess should protect us against gross overruns.
> 
> I'm not keen on the size information, it seems superfluous.

Are you referring to the fact that fp will point to the end of
extra_context, or simply that we don't really need to know the size?


The first is only true of the signal frame.  For a sigcontext on the
heap (e.g., getcontext/setcontext) the frame record has no role and
nothing would point to it, so it's no use for locating the end of
extra_context.  In practice there won't be a frame record in this
situation.

The second is a valid point, just as we don't need to know the size of
the destination buffer of strcpy() or sprintf().  The programmer can
ensure that it's big enough.  Moreover, from the kernel we're also
protected by uaccess.

It's easy to screw up here though.  setcontext assumes that mcontext_t
is fixed-size, so if there is no embedded size information then there
is no way to protect against overruns.  In userspace, there is no
uaccess protection and we can simply walk across the end of a heap
block etc.

> BTW, does SIGFRAME_MAXSZ now become ABI? Or the user only needs to
> interrogate the frame size and we keep this internal to the kernel?

If the kernel rejects extra_contexts that cause this limit to be
exceeded, then yes -- though it will rarely be relevant except in the
case of memory corruption, or if architecture extensions eventually
require a larger frame.

(sve_context could theoretically grow larger then SIGFRAME_MAXSZ all by
itself, but that's unlikely to happen any time soon.)


Userspace could hit SIGFRAME_MAXSZ by constructing a valid sequence of
records that is ridiculously large, by padding out the records: common
sense suggests not to do this, but it's never been documented or
enforced.  I didn't feel comfortable changing the behaviour here to be
more strict.


So, SIGFRAME_MAXSZ should either be given a larger, more future-proof
value ... or otherwise we should perhaps get rid of it entirely.

?

Cheers
---Dave

[RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Dave Martin]

On Tue, May 23, 2017 at 12:30:19PM +0100, Catalin Marinas wrote:
> On Mon, May 15, 2017 at 02:24:45PM +0100, Dave P Martin wrote:
> > On Fri, May 12, 2017 at 05:57:24PM +0100, Catalin Marinas wrote:
> > > On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:

[...]

> > Originally I preferred not to waste the space and did move fp/lr to the
> > end, but someone (I think you or Will) expressed concern that the fp/lr
> > position relative to the signal frame _might_ count as ABI.
> > 
> > I think it's not that likely that software will be relying on this,
> > since it appears easier just to follow the frame chain than to treat
> > this as a special case.
> > 
> > But it's hard to be certain.  It comes down to a judgement call.
> 
> I would not consider this ABI. The ABI part is that the fp register
> points to where fp/lr were saved.

Fair enough, I'm happy to move it back.  (That was my preferred design
in the first place anyway).

> > > > +#define EXTRA_MAGIC	0x45585401
> > > > +
> > > > +struct extra_context {
> > > > +	struct _aarch64_ctx head;
> > > > +	void __user *data;	/* 16-byte aligned pointer to extra space */
> > > "__user" is a kernel-only attribute, we shouldn't expose it in a uapi
> > > header.
> > 
> > This is filtered out by headers_install, just like #ifdef __KERNEL__.
> 
> Ah, ok, I missed this.

It was a surprise to me too...

> > > > +	__u32 size;		/* size in bytes of the extra space */
> > > > +};
> > > 
> > > Do we need the size of the extra space? Can we not infer it anyway by
> > > walking the contexts save there? Surely we don't expect more than one
> > > extra context.
> > 
> > Strictly speaking we don't need it.  When userspace parses a signal
> > frame generated by the kernel, it can trust the kernel to write a well-
> > formed signal frame.
> > 
> > In sigreturn it allows us to retain a sanity-check on overall size
> > similar to what sizeof(__reserved) gives us.  This "feels cleaner"
> > to me, but the value of it is debatable, since we can still apply
> > SIGFRAME_MAXSZ and uaccess should protect us against gross overruns.
> 
> I'm not keen on the size information, it seems superfluous.

Are you referring to the fact that fp will point to the end of
extra_context, or simply that we don't really need to know the size?


The first is only true of the signal frame.  For a sigcontext on the
heap (e.g., getcontext/setcontext) the frame record has no role and
nothing would point to it, so it's no use for locating the end of
extra_context.  In practice there won't be a frame record in this
situation.

The second is a valid point, just as we don't need to know the size of
the destination buffer of strcpy() or sprintf().  The programmer can
ensure that it's big enough.  Moreover, from the kernel we're also
protected by uaccess.

It's easy to screw up here though.  setcontext assumes that mcontext_t
is fixed-size, so if there is no embedded size information then there
is no way to protect against overruns.  In userspace, there is no
uaccess protection and we can simply walk across the end of a heap
block etc.

> BTW, does SIGFRAME_MAXSZ now become ABI? Or the user only needs to
> interrogate the frame size and we keep this internal to the kernel?

If the kernel rejects extra_contexts that cause this limit to be
exceeded, then yes -- though it will rarely be relevant except in the
case of memory corruption, or if architecture extensions eventually
require a larger frame.

(sve_context could theoretically grow larger then SIGFRAME_MAXSZ all by
itself, but that's unlikely to happen any time soon.)


Userspace could hit SIGFRAME_MAXSZ by constructing a valid sequence of
records that is ridiculously large, by padding out the records: common
sense suggests not to do this, but it's never been documented or
enforced.  I didn't feel comfortable changing the behaviour here to be
more strict.


So, SIGFRAME_MAXSZ should either be given a larger, more future-proof
value ... or otherwise we should perhaps get rid of it entirely.

?

Cheers
---Dave

Re: [RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Catalin Marinas]

On Fri, May 26, 2017 at 12:37:32PM +0100, Dave P Martin wrote:
> On Tue, May 23, 2017 at 12:30:19PM +0100, Catalin Marinas wrote:
> > On Mon, May 15, 2017 at 02:24:45PM +0100, Dave P Martin wrote:
> > > On Fri, May 12, 2017 at 05:57:24PM +0100, Catalin Marinas wrote:
> > > > On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:
> > > > > +	__u32 size;		/* size in bytes of the extra space */
> > > > > +};
> > > > 
> > > > Do we need the size of the extra space? Can we not infer it anyway by
> > > > walking the contexts save there? Surely we don't expect more than one
> > > > extra context.
> > > 
> > > Strictly speaking we don't need it.  When userspace parses a signal
> > > frame generated by the kernel, it can trust the kernel to write a well-
> > > formed signal frame.
> > > 
> > > In sigreturn it allows us to retain a sanity-check on overall size
> > > similar to what sizeof(__reserved) gives us.  This "feels cleaner"
> > > to me, but the value of it is debatable, since we can still apply
> > > SIGFRAME_MAXSZ and uaccess should protect us against gross overruns.
> > 
> > I'm not keen on the size information, it seems superfluous.
> 
> Are you referring to the fact that fp will point to the end of
> extra_context, or simply that we don't really need to know the size?

The latter, I don't think we need to know the size explicitly, we can
add up the chained structures to calculate it.

> The second is a valid point, just as we don't need to know the size of
> the destination buffer of strcpy() or sprintf().  The programmer can
> ensure that it's big enough.  Moreover, from the kernel we're also
> protected by uaccess.
> 
> It's easy to screw up here though.  setcontext assumes that mcontext_t
> is fixed-size, so if there is no embedded size information then there
> is no way to protect against overruns.  In userspace, there is no
> uaccess protection and we can simply walk across the end of a heap
> block etc.

So you intend size to be the maximum size, not the actual size of the
chained contexts? If that's the case, I think we can keep it, only that
I'd rather have the time size_t or __u64 to avoid implicit padding.

> > BTW, does SIGFRAME_MAXSZ now become ABI? Or the user only needs to
> > interrogate the frame size and we keep this internal to the kernel?
> 
> If the kernel rejects extra_contexts that cause this limit to be
> exceeded, then yes -- though it will rarely be relevant except in the
> case of memory corruption, or if architecture extensions eventually
> require a larger frame.
> 
> (sve_context could theoretically grow larger then SIGFRAME_MAXSZ all by
> itself, but that's unlikely to happen any time soon.)
> 
> Userspace could hit SIGFRAME_MAXSZ by constructing a valid sequence of
> records that is ridiculously large, by padding out the records: common
> sense suggests not to do this, but it's never been documented or
> enforced.  I didn't feel comfortable changing the behaviour here to be
> more strict.
> 
> So, SIGFRAME_MAXSZ should either be given a larger, more future-proof
> value ... or otherwise we should perhaps get rid of it entirely.

If we can, yes, I would get rid of it.

-- 
Catalin

[RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Catalin Marinas]

On Fri, May 26, 2017 at 12:37:32PM +0100, Dave P Martin wrote:
> On Tue, May 23, 2017 at 12:30:19PM +0100, Catalin Marinas wrote:
> > On Mon, May 15, 2017 at 02:24:45PM +0100, Dave P Martin wrote:
> > > On Fri, May 12, 2017 at 05:57:24PM +0100, Catalin Marinas wrote:
> > > > On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:
> > > > > +	__u32 size;		/* size in bytes of the extra space */
> > > > > +};
> > > > 
> > > > Do we need the size of the extra space? Can we not infer it anyway by
> > > > walking the contexts save there? Surely we don't expect more than one
> > > > extra context.
> > > 
> > > Strictly speaking we don't need it.  When userspace parses a signal
> > > frame generated by the kernel, it can trust the kernel to write a well-
> > > formed signal frame.
> > > 
> > > In sigreturn it allows us to retain a sanity-check on overall size
> > > similar to what sizeof(__reserved) gives us.  This "feels cleaner"
> > > to me, but the value of it is debatable, since we can still apply
> > > SIGFRAME_MAXSZ and uaccess should protect us against gross overruns.
> > 
> > I'm not keen on the size information, it seems superfluous.
> 
> Are you referring to the fact that fp will point to the end of
> extra_context, or simply that we don't really need to know the size?

The latter, I don't think we need to know the size explicitly, we can
add up the chained structures to calculate it.

> The second is a valid point, just as we don't need to know the size of
> the destination buffer of strcpy() or sprintf().  The programmer can
> ensure that it's big enough.  Moreover, from the kernel we're also
> protected by uaccess.
> 
> It's easy to screw up here though.  setcontext assumes that mcontext_t
> is fixed-size, so if there is no embedded size information then there
> is no way to protect against overruns.  In userspace, there is no
> uaccess protection and we can simply walk across the end of a heap
> block etc.

So you intend size to be the maximum size, not the actual size of the
chained contexts? If that's the case, I think we can keep it, only that
I'd rather have the time size_t or __u64 to avoid implicit padding.

> > BTW, does SIGFRAME_MAXSZ now become ABI? Or the user only needs to
> > interrogate the frame size and we keep this internal to the kernel?
> 
> If the kernel rejects extra_contexts that cause this limit to be
> exceeded, then yes -- though it will rarely be relevant except in the
> case of memory corruption, or if architecture extensions eventually
> require a larger frame.
> 
> (sve_context could theoretically grow larger then SIGFRAME_MAXSZ all by
> itself, but that's unlikely to happen any time soon.)
> 
> Userspace could hit SIGFRAME_MAXSZ by constructing a valid sequence of
> records that is ridiculously large, by padding out the records: common
> sense suggests not to do this, but it's never been documented or
> enforced.  I didn't feel comfortable changing the behaviour here to be
> more strict.
> 
> So, SIGFRAME_MAXSZ should either be given a larger, more future-proof
> value ... or otherwise we should perhaps get rid of it entirely.

If we can, yes, I would get rid of it.

-- 
Catalin

Re: [RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Dave Martin]

On Mon, Jun 05, 2017 at 03:17:44PM +0100, Catalin Marinas wrote:
> On Fri, May 26, 2017 at 12:37:32PM +0100, Dave P Martin wrote:
> > On Tue, May 23, 2017 at 12:30:19PM +0100, Catalin Marinas wrote:
> > > On Mon, May 15, 2017 at 02:24:45PM +0100, Dave P Martin wrote:
> > > > On Fri, May 12, 2017 at 05:57:24PM +0100, Catalin Marinas wrote:
> > > > > On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:
> > > > > > +	__u32 size;		/* size in bytes of the extra space */
> > > > > > +};
> > > > > 
> > > > > Do we need the size of the extra space? Can we not infer it anyway by
> > > > > walking the contexts save there? Surely we don't expect more than one
> > > > > extra context.
> > > > 
> > > > Strictly speaking we don't need it.  When userspace parses a signal
> > > > frame generated by the kernel, it can trust the kernel to write a well-
> > > > formed signal frame.
> > > > 
> > > > In sigreturn it allows us to retain a sanity-check on overall size
> > > > similar to what sizeof(__reserved) gives us.  This "feels cleaner"
> > > > to me, but the value of it is debatable, since we can still apply
> > > > SIGFRAME_MAXSZ and uaccess should protect us against gross overruns.
> > > 
> > > I'm not keen on the size information, it seems superfluous.
> > 
> > Are you referring to the fact that fp will point to the end of
> > extra_context, or simply that we don't really need to know the size?
> 
> The latter, I don't think we need to know the size explicitly, we can
> add up the chained structures to calculate it.
> 
> > The second is a valid point, just as we don't need to know the size of
> > the destination buffer of strcpy() or sprintf().  The programmer can
> > ensure that it's big enough.  Moreover, from the kernel we're also
> > protected by uaccess.
> > 
> > It's easy to screw up here though.  setcontext assumes that mcontext_t
> > is fixed-size, so if there is no embedded size information then there
> > is no way to protect against overruns.  In userspace, there is no
> > uaccess protection and we can simply walk across the end of a heap
> > block etc.
> 
> So you intend size to be the maximum size, not the actual size of the

Yes, except that when the signal frame is generated by the kernel those
will be the same, since the kernel generates a frame that is only as
large as needed for the records dumped.

I'm thinking of the case where userspace, or a debugger, modifies a
frame obtained from a signal, or similar.  This might involve deleting
or resizing records.  Encoding the original size of the frame may help
code defend against overrun bugs, though obviously it's no guarantee.

> chained contexts? If that's the case, I think we can keep it, only that
> I'd rather have the time size_t or __u64 to avoid implicit padding.

Sure, it can be a u64.  I wanted to avoid the suggestion that the frame
should be that large, but 32 bits already allows it to be crazy large
anyway, so I don't think making it 32-bit helps.

> > > BTW, does SIGFRAME_MAXSZ now become ABI? Or the user only needs to
> > > interrogate the frame size and we keep this internal to the kernel?
> > 
> > If the kernel rejects extra_contexts that cause this limit to be
> > exceeded, then yes -- though it will rarely be relevant except in the
> > case of memory corruption, or if architecture extensions eventually
> > require a larger frame.
> > 
> > (sve_context could theoretically grow larger then SIGFRAME_MAXSZ all by
> > itself, but that's unlikely to happen any time soon.)
> > 
> > Userspace could hit SIGFRAME_MAXSZ by constructing a valid sequence of
> > records that is ridiculously large, by padding out the records: common
> > sense suggests not to do this, but it's never been documented or
> > enforced.  I didn't feel comfortable changing the behaviour here to be
> > more strict.
> > 
> > So, SIGFRAME_MAXSZ should either be given a larger, more future-proof
> > value ... or otherwise we should perhaps get rid of it entirely.
> 
> If we can, yes, I would get rid of it.

If the size field is retained I prefer to keep this, but it's
deliberately not in any header.  This allows the kernel to have a
stricter idea about what is sane, without it formally being ABI.

This is supposed to be a deterrent against people writing signal frame
code manipulation code in a stupid way.  SIGFRAME_MAXSZ should only
ever be increased during maintenance -- it's probably worth adding a
comment on that point.

Cheers
---Dave

[RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Dave Martin]

On Mon, Jun 05, 2017 at 03:17:44PM +0100, Catalin Marinas wrote:
> On Fri, May 26, 2017 at 12:37:32PM +0100, Dave P Martin wrote:
> > On Tue, May 23, 2017 at 12:30:19PM +0100, Catalin Marinas wrote:
> > > On Mon, May 15, 2017 at 02:24:45PM +0100, Dave P Martin wrote:
> > > > On Fri, May 12, 2017 at 05:57:24PM +0100, Catalin Marinas wrote:
> > > > > On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:
> > > > > > +	__u32 size;		/* size in bytes of the extra space */
> > > > > > +};
> > > > > 
> > > > > Do we need the size of the extra space? Can we not infer it anyway by
> > > > > walking the contexts save there? Surely we don't expect more than one
> > > > > extra context.
> > > > 
> > > > Strictly speaking we don't need it.  When userspace parses a signal
> > > > frame generated by the kernel, it can trust the kernel to write a well-
> > > > formed signal frame.
> > > > 
> > > > In sigreturn it allows us to retain a sanity-check on overall size
> > > > similar to what sizeof(__reserved) gives us.  This "feels cleaner"
> > > > to me, but the value of it is debatable, since we can still apply
> > > > SIGFRAME_MAXSZ and uaccess should protect us against gross overruns.
> > > 
> > > I'm not keen on the size information, it seems superfluous.
> > 
> > Are you referring to the fact that fp will point to the end of
> > extra_context, or simply that we don't really need to know the size?
> 
> The latter, I don't think we need to know the size explicitly, we can
> add up the chained structures to calculate it.
> 
> > The second is a valid point, just as we don't need to know the size of
> > the destination buffer of strcpy() or sprintf().  The programmer can
> > ensure that it's big enough.  Moreover, from the kernel we're also
> > protected by uaccess.
> > 
> > It's easy to screw up here though.  setcontext assumes that mcontext_t
> > is fixed-size, so if there is no embedded size information then there
> > is no way to protect against overruns.  In userspace, there is no
> > uaccess protection and we can simply walk across the end of a heap
> > block etc.
> 
> So you intend size to be the maximum size, not the actual size of the

Yes, except that when the signal frame is generated by the kernel those
will be the same, since the kernel generates a frame that is only as
large as needed for the records dumped.

I'm thinking of the case where userspace, or a debugger, modifies a
frame obtained from a signal, or similar.  This might involve deleting
or resizing records.  Encoding the original size of the frame may help
code defend against overrun bugs, though obviously it's no guarantee.

> chained contexts? If that's the case, I think we can keep it, only that
> I'd rather have the time size_t or __u64 to avoid implicit padding.

Sure, it can be a u64.  I wanted to avoid the suggestion that the frame
should be that large, but 32 bits already allows it to be crazy large
anyway, so I don't think making it 32-bit helps.

> > > BTW, does SIGFRAME_MAXSZ now become ABI? Or the user only needs to
> > > interrogate the frame size and we keep this internal to the kernel?
> > 
> > If the kernel rejects extra_contexts that cause this limit to be
> > exceeded, then yes -- though it will rarely be relevant except in the
> > case of memory corruption, or if architecture extensions eventually
> > require a larger frame.
> > 
> > (sve_context could theoretically grow larger then SIGFRAME_MAXSZ all by
> > itself, but that's unlikely to happen any time soon.)
> > 
> > Userspace could hit SIGFRAME_MAXSZ by constructing a valid sequence of
> > records that is ridiculously large, by padding out the records: common
> > sense suggests not to do this, but it's never been documented or
> > enforced.  I didn't feel comfortable changing the behaviour here to be
> > more strict.
> > 
> > So, SIGFRAME_MAXSZ should either be given a larger, more future-proof
> > value ... or otherwise we should perhaps get rid of it entirely.
> 
> If we can, yes, I would get rid of it.

If the size field is retained I prefer to keep this, but it's
deliberately not in any header.  This allows the kernel to have a
stricter idea about what is sane, without it formally being ABI.

This is supposed to be a deterrent against people writing signal frame
code manipulation code in a stupid way.  SIGFRAME_MAXSZ should only
ever be increased during maintenance -- it's probably worth adding a
comment on that point.

Cheers
---Dave

Re: [RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Dave Martin]

On Tue, Jun 06, 2017 at 12:37:53PM +0100, Dave Martin wrote:
> On Mon, Jun 05, 2017 at 03:17:44PM +0100, Catalin Marinas wrote:

[...]

[extra_context.size]

> > I'd rather have the time size_t or __u64 to avoid implicit padding.
> 
> Sure, it can be a u64.  I wanted to avoid the suggestion that the frame
> should be that large, but 32 bits already allows it to be crazy large
> anyway, so I don't think making it 32-bit helps.

Actually, there is still implicit padding even with u64, since the total
size is 16 bytes + sizeof(extra_context.size).

Since u64 is much bigger then we'd ever want, and to avoid introducing
new bugs, do you object to keeping size as u32 and adding explicit
padding instead?

Cheers
---Dave

[RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Dave Martin]

On Tue, Jun 06, 2017 at 12:37:53PM +0100, Dave Martin wrote:
> On Mon, Jun 05, 2017 at 03:17:44PM +0100, Catalin Marinas wrote:

[...]

[extra_context.size]

> > I'd rather have the time size_t or __u64 to avoid implicit padding.
> 
> Sure, it can be a u64.  I wanted to avoid the suggestion that the frame
> should be that large, but 32 bits already allows it to be crazy large
> anyway, so I don't think making it 32-bit helps.

Actually, there is still implicit padding even with u64, since the total
size is 16 bytes + sizeof(extra_context.size).

Since u64 is much bigger then we'd ever want, and to avoid introducing
new bugs, do you object to keeping size as u32 and adding explicit
padding instead?

Cheers
---Dave

Re: [RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Catalin Marinas]

On Tue, Jun 06, 2017 at 02:58:18PM +0100, Dave P Martin wrote:
> On Tue, Jun 06, 2017 at 12:37:53PM +0100, Dave Martin wrote:
> > On Mon, Jun 05, 2017 at 03:17:44PM +0100, Catalin Marinas wrote:
> 
> [...]
> 
> [extra_context.size]
> 
> > > I'd rather have the time size_t or __u64 to avoid implicit padding.
> > 
> > Sure, it can be a u64.  I wanted to avoid the suggestion that the frame
> > should be that large, but 32 bits already allows it to be crazy large
> > anyway, so I don't think making it 32-bit helps.
> 
> Actually, there is still implicit padding even with u64, since the total
> size is 16 bytes + sizeof(extra_context.size).
> 
> Since u64 is much bigger then we'd ever want, and to avoid introducing
> new bugs, do you object to keeping size as u32 and adding explicit
> padding instead?

I missed the extra padding to 16-byte alignment. In this case, I'm fine
with u32.

-- 
Catalin

[RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Catalin Marinas]

On Tue, Jun 06, 2017 at 02:58:18PM +0100, Dave P Martin wrote:
> On Tue, Jun 06, 2017 at 12:37:53PM +0100, Dave Martin wrote:
> > On Mon, Jun 05, 2017 at 03:17:44PM +0100, Catalin Marinas wrote:
> 
> [...]
> 
> [extra_context.size]
> 
> > > I'd rather have the time size_t or __u64 to avoid implicit padding.
> > 
> > Sure, it can be a u64.  I wanted to avoid the suggestion that the frame
> > should be that large, but 32 bits already allows it to be crazy large
> > anyway, so I don't think making it 32-bit helps.
> 
> Actually, there is still implicit padding even with u64, since the total
> size is 16 bytes + sizeof(extra_context.size).
> 
> Since u64 is much bigger then we'd ever want, and to avoid introducing
> new bugs, do you object to keeping size as u32 and adding explicit
> padding instead?

I missed the extra padding to 16-byte alignment. In this case, I'm fine
with u32.

-- 
Catalin

Re: [RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Catalin Marinas]

On Tue, Jun 06, 2017 at 12:37:53PM +0100, Dave P Martin wrote:
> On Mon, Jun 05, 2017 at 03:17:44PM +0100, Catalin Marinas wrote:
> > On Fri, May 26, 2017 at 12:37:32PM +0100, Dave P Martin wrote:
> > > On Tue, May 23, 2017 at 12:30:19PM +0100, Catalin Marinas wrote:
> > > > BTW, does SIGFRAME_MAXSZ now become ABI? Or the user only needs to
> > > > interrogate the frame size and we keep this internal to the kernel?
> > > 
> > > If the kernel rejects extra_contexts that cause this limit to be
> > > exceeded, then yes -- though it will rarely be relevant except in the
> > > case of memory corruption, or if architecture extensions eventually
> > > require a larger frame.
> > > 
> > > (sve_context could theoretically grow larger then SIGFRAME_MAXSZ all by
> > > itself, but that's unlikely to happen any time soon.)
> > > 
> > > Userspace could hit SIGFRAME_MAXSZ by constructing a valid sequence of
> > > records that is ridiculously large, by padding out the records: common
> > > sense suggests not to do this, but it's never been documented or
> > > enforced.  I didn't feel comfortable changing the behaviour here to be
> > > more strict.
> > > 
> > > So, SIGFRAME_MAXSZ should either be given a larger, more future-proof
> > > value ... or otherwise we should perhaps get rid of it entirely.
> > 
> > If we can, yes, I would get rid of it.
> 
> If the size field is retained I prefer to keep this, but it's
> deliberately not in any header.  This allows the kernel to have a
> stricter idea about what is sane, without it formally being ABI.
> 
> This is supposed to be a deterrent against people writing signal frame
> code manipulation code in a stupid way.  SIGFRAME_MAXSZ should only
> ever be increased during maintenance -- it's probably worth adding a
> comment on that point.

Fine by me.

-- 
Catalin

[RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Catalin Marinas]

On Tue, Jun 06, 2017 at 12:37:53PM +0100, Dave P Martin wrote:
> On Mon, Jun 05, 2017 at 03:17:44PM +0100, Catalin Marinas wrote:
> > On Fri, May 26, 2017 at 12:37:32PM +0100, Dave P Martin wrote:
> > > On Tue, May 23, 2017 at 12:30:19PM +0100, Catalin Marinas wrote:
> > > > BTW, does SIGFRAME_MAXSZ now become ABI? Or the user only needs to
> > > > interrogate the frame size and we keep this internal to the kernel?
> > > 
> > > If the kernel rejects extra_contexts that cause this limit to be
> > > exceeded, then yes -- though it will rarely be relevant except in the
> > > case of memory corruption, or if architecture extensions eventually
> > > require a larger frame.
> > > 
> > > (sve_context could theoretically grow larger then SIGFRAME_MAXSZ all by
> > > itself, but that's unlikely to happen any time soon.)
> > > 
> > > Userspace could hit SIGFRAME_MAXSZ by constructing a valid sequence of
> > > records that is ridiculously large, by padding out the records: common
> > > sense suggests not to do this, but it's never been documented or
> > > enforced.  I didn't feel comfortable changing the behaviour here to be
> > > more strict.
> > > 
> > > So, SIGFRAME_MAXSZ should either be given a larger, more future-proof
> > > value ... or otherwise we should perhaps get rid of it entirely.
> > 
> > If we can, yes, I would get rid of it.
> 
> If the size field is retained I prefer to keep this, but it's
> deliberately not in any header.  This allows the kernel to have a
> stricter idea about what is sane, without it formally being ABI.
> 
> This is supposed to be a deterrent against people writing signal frame
> code manipulation code in a stupid way.  SIGFRAME_MAXSZ should only
> ever be increased during maintenance -- it's probably worth adding a
> comment on that point.

Fine by me.

-- 
Catalin

Re: [RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Dave Martin]

On Tue, May 23, 2017 at 12:30:19PM +0100, Catalin Marinas wrote:
> On Mon, May 15, 2017 at 02:24:45PM +0100, Dave P Martin wrote:
> > On Fri, May 12, 2017 at 05:57:24PM +0100, Catalin Marinas wrote:
> > > On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:
> > > > --- a/arch/arm64/include/uapi/asm/sigcontext.h
> > > > +++ b/arch/arm64/include/uapi/asm/sigcontext.h
> > > > @@ -80,4 +80,31 @@ struct esr_context {
> > > >  	__u64 esr;
> > > >  };
> > > >  
> > > > +/*
> > > > + * Pointer to extra space for additional structures that don't fit in
> > > > + * sigcontext.__reserved[].  Note:
> > > > + *
> > > > + * 1) fpsimd_context, esr_context and extra_context must be placed in
> > > > + * sigcontext.__reserved[] if present.  They cannot be placed in the
> > > > + * extra space.  Any other record can be placed either in the extra
> > > > + * space or in sigcontext.__reserved[].
> > > > + *
> > > > + * 2) There must not be more than one extra_context.
> > > > + *
> > > > + * 3) If extra_context is present, it must be followed immediately in
> > > > + * sigcontext.__reserved[] by the terminating null _aarch64_ctx (i.e.,
> > > > + * extra_context must be the last record in sigcontext.__reserved[]
> > > > + * except for the terminator).
> > > > + *
> > > > + * 4) The extra space must itself be terminated with a null
> > > > + * _aarch64_ctx.
> > > > + */
> > > 
> > > IIUC, if we need to save some state that doesn't fit in what's left of
> > > sigcontext.__reserved[] (e.g. SVE with 1024-bit vector length), we
> > > ignore the available space and go for a memory block following the end
> > > of sigcontext.__reserved[] + 16. Is there a reason we can't store the
> > > new state across the end of sigcontext.__reserved[] and move fp/lr at
> > > the end of the new frame? I'm not sure the fp/lr position immediately
> > > after __reserved[] counts as ABI.
> > 
> > This was my original view.
> > 
> > Originally I preferred not to waste the space and did move fp/lr to the
> > end, but someone (I think you or Will) expressed concern that the fp/lr
> > position relative to the signal frame _might_ count as ABI.
> > 
> > I think it's not that likely that software will be relying on this,
> > since it appears easier just to follow the frame chain than to treat
> > this as a special case.
> > 
> > But it's hard to be certain.  It comes down to a judgement call.
> 
> I would not consider this ABI. The ABI part is that the fp register
> points to where fp/lr were saved.

On this point, it looks like the libgcc unwinder

https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libgcc/config/aarch64/linux-unwind.h;h=d5d6980442fd47b1f1e499e99cb25b5fffbdbeb3;hb=HEAD

doesn't rely on the frame record location.  It clones the (internal)
struct rt_sigframe definition from v3.7, which doesn't include any frame
record, and mines fp and lr out of the signal frame AFAICT.

It appears that gdb and libunwind likely take the same approach, but
I've not looked closely yet.

The frame record in rt_sigframe was added in by Will in 304ef4e83672
("arm64: signal: push the unwinding prologue on the signal stack"),
which changes from pushing the frame record onto the interrupted stack
(which may be inaccessible for a SEGV), to pushing onto the signal
stack.


Even with the frame record split from rt_sigframe, I've not seen any
failed backtrace in gdb.  Throwing an exception from a SEGV handler in
C++ (with -fnon-call-exceptions) also appears to work reliably with
that change, even when the signal frame grows.

In any case, there is no ABI break unless there is extra_context, so
it shouldn't impact current userspace.

Cheers
---Dave

[RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed

[Dave Martin]

On Tue, May 23, 2017 at 12:30:19PM +0100, Catalin Marinas wrote:
> On Mon, May 15, 2017 at 02:24:45PM +0100, Dave P Martin wrote:
> > On Fri, May 12, 2017 at 05:57:24PM +0100, Catalin Marinas wrote:
> > > On Wed, Apr 12, 2017 at 06:01:13PM +0100, Dave P Martin wrote:
> > > > --- a/arch/arm64/include/uapi/asm/sigcontext.h
> > > > +++ b/arch/arm64/include/uapi/asm/sigcontext.h
> > > > @@ -80,4 +80,31 @@ struct esr_context {
> > > >  	__u64 esr;
> > > >  };
> > > >  
> > > > +/*
> > > > + * Pointer to extra space for additional structures that don't fit in
> > > > + * sigcontext.__reserved[].  Note:
> > > > + *
> > > > + * 1) fpsimd_context, esr_context and extra_context must be placed in
> > > > + * sigcontext.__reserved[] if present.  They cannot be placed in the
> > > > + * extra space.  Any other record can be placed either in the extra
> > > > + * space or in sigcontext.__reserved[].
> > > > + *
> > > > + * 2) There must not be more than one extra_context.
> > > > + *
> > > > + * 3) If extra_context is present, it must be followed immediately in
> > > > + * sigcontext.__reserved[] by the terminating null _aarch64_ctx (i.e.,
> > > > + * extra_context must be the last record in sigcontext.__reserved[]
> > > > + * except for the terminator).
> > > > + *
> > > > + * 4) The extra space must itself be terminated with a null
> > > > + * _aarch64_ctx.
> > > > + */
> > > 
> > > IIUC, if we need to save some state that doesn't fit in what's left of
> > > sigcontext.__reserved[] (e.g. SVE with 1024-bit vector length), we
> > > ignore the available space and go for a memory block following the end
> > > of sigcontext.__reserved[] + 16. Is there a reason we can't store the
> > > new state across the end of sigcontext.__reserved[] and move fp/lr at
> > > the end of the new frame? I'm not sure the fp/lr position immediately
> > > after __reserved[] counts as ABI.
> > 
> > This was my original view.
> > 
> > Originally I preferred not to waste the space and did move fp/lr to the
> > end, but someone (I think you or Will) expressed concern that the fp/lr
> > position relative to the signal frame _might_ count as ABI.
> > 
> > I think it's not that likely that software will be relying on this,
> > since it appears easier just to follow the frame chain than to treat
> > this as a special case.
> > 
> > But it's hard to be certain.  It comes down to a judgement call.
> 
> I would not consider this ABI. The ABI part is that the fp register
> points to where fp/lr were saved.

On this point, it looks like the libgcc unwinder

https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libgcc/config/aarch64/linux-unwind.h;h=d5d6980442fd47b1f1e499e99cb25b5fffbdbeb3;hb=HEAD

doesn't rely on the frame record location.  It clones the (internal)
struct rt_sigframe definition from v3.7, which doesn't include any frame
record, and mines fp and lr out of the signal frame AFAICT.

It appears that gdb and libunwind likely take the same approach, but
I've not looked closely yet.

The frame record in rt_sigframe was added in by Will in 304ef4e83672
("arm64: signal: push the unwinding prologue on the signal stack"),
which changes from pushing the frame record onto the interrupted stack
(which may be inaccessible for a SEGV), to pushing onto the signal
stack.


Even with the frame record split from rt_sigframe, I've not seen any
failed backtrace in gdb.  Throwing an exception from a SEGV handler in
C++ (with -fnon-call-exceptions) also appears to work reliably with
that change, even when the signal frame grows.

In any case, there is no ABI break unless there is extra_context, so
it shouldn't impact current userspace.

Cheers
---Dave