siginfo fixes/cleanups esp SI_USER

While reviewing the signal sending on openrisc the do_unaligned_access function stood out because it is obviously wrong. A comment about an si_code set above when actually si_code is never set. Leading to a random si_code being sent to userspace in the event of an unaligned access.

Looking further SIGBUS BUS_ADRALN is the proper pair of signal and si_code to send for an unaligned access. That is what other architectures do and what is required by posix.

Given that do_unaligned_access is broken in a way that no one can be relying on it on openrisc fix the code to just do the right thing.

Cc [email protected]

Fixes 769a8a96229e ("OpenRISC: Traps")

Cc Jonas Bonn <[email protected]>

Cc Stefan Kristiansson <[email protected]>

Cc Stafford Horne <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc [email protected]

Signed-off-by "Eric W. Biederman" <[email protected]>

---
 arch/openrisc/kernel/traps.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

arch/openrisc/kernel/traps.c +5 -5

index 4085d72fa5ae..9e38dc66c9e4 100644
@@ -266,12 +266,12 @@ asmlinkage void do_unaligned_access(struct pt_regs *regs, unsigned long address)
 	siginfo_t info;
 
 	if (user_mode(regs)) {
-		/* Send a SIGSEGV */
-		info.si_signo = SIGSEGV;
+		/* Send a SIGBUS */
+		info.si_signo = SIGBUS;
 		info.si_errno = 0;
-		/* info.si_code has been set above */
-		info.si_addr = (void *)address;
-		force_sig_info(SIGSEGV, &info, current);
+		info.si_code = BUS_ADRALN;
+		info.si_addr = (void __user *)address;
+		force_sig_info(SIGBUS, &info, current);
 	} else {
 		printk("KERNEL: Unaligned Access 0x%.8lx\n", address);
 		show_registers(regs);

Signature · 1 line

2.14.1

Quoted 6 lines · Eric W. Biederman

While reviewing the signal sending on openrisc the do_unaligned_access function stood out because it is obviously wrong. A comment about an si_code set above when actually si_code is never set. Leading to a random si_code being sent to userspace in the event of an unaligned access.

Quoted 6 lines

Looking further SIGBUS BUS_ADRALN is the proper pair of signal and si_code to send for an unaligned access. That is what other architectures do and what is required by posix.

Given that do_unaligned_access is broken in a way that no one can be relying on it on openrisc fix the code to just do the right thing.

Thanks, this looks good to me.

Acked-by Stafford Horne <[email protected]>

I see you have a series of related issues, so I guess you want to get them merged together. Let me know if I should put this patch onto my queue seperately.

Trivia: this looks to have been copied from the mm page fault handling code, hence the strange comment.

  $ grep -r "info.si_code has been set above" arch/
  arch/cris/mm/fault.c:           /* info.si_code has been set above */
  arch/m32r/mm/fault.c:           /* info.si_code has been set above */
  arch/mn10300/mm/fault.c:                /* info.si_code has been set above */
  arch/openrisc/mm/fault.c:               /* info.si_code has been set above */
  arch/openrisc/kernel/traps.c:           /* info.si_code has been set above */
  arch/arc/mm/fault.c:            /* info.si_code has been set above */
  arch/xtensa/mm/fault.c:         /* info.si_code has been set above */
  arch/mips/mm/fault.c:           /* info.si_code has been set above */
  arch/score/mm/fault.c:          /* info.si_code has been set above */
  arch/frv/mm/fault.c:            /* info.si_code has been set above */

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Utilizing FPE_FIXME siginfo_layout will now return SIL_FAULT and the appropriate fields will reliably be copied.

This bug is 13 years old and parsic machines are no longer being built so I don't know if it possible or worth fixing it. But it is at least worth documenting this so other architectures don't make the same mistake.

* Eric W. Biederman <[email protected]>:

Quoted 11 lines

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Quoted 7 lines

Utilizing FPE_FIXME siginfo_layout will now return SIL_FAULT and the appropriate fields will reliably be copied.

This bug is 13 years old and parsic machines are no longer being built so I don't know if it possible or worth fixing it. But it is at least worth documenting this so other architectures don't make the same mistake.

I think we should fix it, even if we now break the ABI.

It's about a "conditional trap" which needs to be handled by userspace. I doubt there is any Linux code out which is utilizing this parisc-specific trap.

I'd suggest to add a new FPE trap si_code (e.g. FPE_CONDTRAP). While at it, maybe we should include the already existing FPE_MDAOVF from the frv architecture, so that arch/frv/include/uapi/asm/siginfo.h can go completely.

Suggested patch is below.

I'm willing to test the patch below on the parisc architecture for a few weeks. And it will break arch/x86/kernel/signal_compat.c which needs looking at then too.

Thoughts?

Helge

[PATCH] parisc: Add FPE_CONDTRAP for conditional trap handling

Posix and common sense requires that SI_USER not be a signal specific si_code. Thus add a new FPE_CONDTRAP si_code for conditional traps.

Signed-off-by Helge Deller <[email protected]>

arch/parisc/kernel/traps.c +3 -3

index 8453724b8009..13702f0f5ba1 100644
@@ -627,9 +627,9 @@ void notrace handle_interruption(int code, struct pt_regs *regs)
 		   on condition  */
 		if(user_mode(regs)){
 			si.si_signo = SIGFPE;
-			/* Set to zero, and let the userspace app figure it out from
-			   the insn pointed to by si_addr */
-			si.si_code = 0;
+			/* Let userspace app figure out from the insn pointed
+			 * to by si_addr */
+			si.si_code = FPE_CONDTRAP;
 			si.si_addr = (void __user *) regs->iaoq[0];
 			force_sig_info(SIGFPE, &si, current);
 			return;

include/uapi/asm-generic/siginfo.h +3 -1

index e447283b8f52..2b759fe42142 100644
@@ -193,7 +193,9 @@ typedef struct siginfo {
 #define FPE_FLTRES	6	/* floating point inexact result */
 #define FPE_FLTINV	7	/* floating point invalid operation */
 #define FPE_FLTSUB	8	/* subscript out of range */
-#define NSIGFPE		8
+#define FPE_MDAOVF	9       /* media overflow */
+#define FPE_CONDTRAP	10      /* trap on condition */
+#define NSIGFPE		10
 
 /*
  * SIGSEGV si_codes

Quoted 34 lines · Helge Deller

* Eric W. Biederman <[email protected]>:

Quoted 11 lines

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Quoted 7 lines

Utilizing FPE_FIXME siginfo_layout will now return SIL_FAULT and the appropriate fields will reliably be copied.

This bug is 13 years old and parsic machines are no longer being built so I don't know if it possible or worth fixing it. But it is at least worth documenting this so other architectures don't make the same mistake.

I think we should fix it, even if we now break the ABI.

It's about a "conditional trap" which needs to be handled by userspace. I doubt there is any Linux code out which is utilizing this parisc-specific trap.

I'd suggest to add a new FPE trap si_code (e.g. FPE_CONDTRAP). While at it, maybe we should include the already existing FPE_MDAOVF from the frv architecture, so that arch/frv/include/uapi/asm/siginfo.h can go completely.

Suggested patch is below.

Quoted 5 lines

I'm willing to test the patch below on the parisc architecture for a few weeks. And it will break arch/x86/kernel/signal_compat.c which needs looking at then too.

Thoughts?

I like it.

* Eric W. Biederman <[email protected]>:

Quoted 11 lines

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Quoted 7 lines

Utilizing FPE_FIXME siginfo_layout will now return SIL_FAULT and the appropriate fields will reliably be copied.

This bug is 13 years old and parsic machines are no longer being built so I don't know if it possible or worth fixing it. But it is at least worth documenting this so other architectures don't make the same mistake.

I think we should fix it, even if we now break the ABI.

It's about a "conditional trap" which needs to be handled by userspace. I doubt there is any Linux code out which is utilizing this parisc-specific trap.

I'd suggest to add a new FPE trap si_code (e.g. FPE_CONDTRAP). While at it, maybe we should include the already existing FPE_MDAOVF from the frv architecture, so that arch/frv/include/uapi/asm/siginfo.h can go completely.

Suggested patch is below.

I'm willing to test the patch below on the parisc architecture for a few weeks. And it will break arch/x86/kernel/signal_compat.c which needs looking at then too.

Quoted 32 lines · Helge Deller

* Eric W. Biederman <[email protected]>:

Quoted 11 lines

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Quoted 7 lines

Utilizing FPE_FIXME siginfo_layout will now return SIL_FAULT and the appropriate fields will reliably be copied.

This bug is 13 years old and parsic machines are no longer being built so I don't know if it possible or worth fixing it. But it is at least worth documenting this so other architectures don't make the same mistake.

I think we should fix it, even if we now break the ABI.

It's about a "conditional trap" which needs to be handled by userspace. I doubt there is any Linux code out which is utilizing this parisc-specific trap.

I'd suggest to add a new FPE trap si_code (e.g. FPE_CONDTRAP). While at it, maybe we should include the already existing FPE_MDAOVF from the frv architecture, so that arch/frv/include/uapi/asm/siginfo.h can go completely.

Quoted 5 lines

Suggested patch is below.

I'm willing to test the patch below on the parisc architecture for a few weeks. And it will break arch/x86/kernel/signal_compat.c which needs looking at then too.

Have you managed to test this change?

I am sitting looking at another new FPE si_code and if this has been tested I figure FPE_CONDTRAP should get the next available FPE si_code and the other change should get the one that follows.

Quoted 36 lines · Eric W. Biederman

Quoted 34 lines · Helge Deller

* Eric W. Biederman <[email protected]>:

Quoted 11 lines

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Quoted 7 lines

Utilizing FPE_FIXME siginfo_layout will now return SIL_FAULT and the appropriate fields will reliably be copied.

This bug is 13 years old and parsic machines are no longer being built so I don't know if it possible or worth fixing it. But it is at least worth documenting this so other architectures don't make the same mistake.

I think we should fix it, even if we now break the ABI.

It's about a "conditional trap" which needs to be handled by userspace. I doubt there is any Linux code out which is utilizing this parisc-specific trap.

I'd suggest to add a new FPE trap si_code (e.g. FPE_CONDTRAP). While at it, maybe we should include the already existing FPE_MDAOVF from the frv architecture, so that arch/frv/include/uapi/asm/siginfo.h can go completely.

Suggested patch is below.

Quoted 5 lines

Quoted 3 lines

I'm willing to test the patch below on the parisc architecture for a few weeks. And it will break arch/x86/kernel/signal_compat.c which needs looking at then too.

Have you managed to test this change?

Sadly I haven't done any further testing yet.

Quoted 4 lines

I am sitting looking at another new FPE si_code and if this has been tested I figure FPE_CONDTRAP should get the next available FPE si_code and the other change should get the one that follows.

I'm fine either way. Do you have a git tree I can pull which includes all your patches? I can then start testing.

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Utilizing FPE_FIXME, BUS_FIXME, TRAP_FIXME siginfo_layout will now return SIL_FAULT and the appropriate fields will be reliably copied.

But folks this is a new and unique kind of bad. This is massively

untested code bad.  This is inventing new and unique was to get
siginfo wrong bad.  This is don't even think about Posix or what
siginfo means bad.  This is lots of eyeballs all missing the fact

that the code does the wrong thing bad. This is getting stuck and keep making the same mistake bad.

I really hope we can find a non userspace breaking fix for this on a port as new as arm64.

Possible ABI fixes include:
- Send the signal without siginfo
- Don't generate a signal

- Possibly assign and use an appropriate si_code
- Don't handle cases which can't happen

arch/arm64/mm/fault.c +0 -0

 	if (esr & ESR_ELx_FnV)
 		info.si_addr = NULL;
 	else

@@ -607,70 +607,70 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 }

arch/arm64/mm/fault.c +1 -1

-	{ do_bad,		SIGBUS,  0,		"unknown 8"			},
+	{ do_bad,		SIGBUS,  BUS_FIXME,	"unknown 8"			},

Quoted 4 lines · Eric W. Biederman

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

I think this situation may have come about because 0 is used as a padding value for "impossible" cases -- i.e., things that can't happen unless the kernel is broken, or things that are too unrecoverable for clean error reporting to be helpful.

In general, I think these values are not expected to reach userspace in practice.

This is not an excuse though -- and not 100% true -- so it's certainly worthy of cleanup.

It would be good to approach this similarly for arm and arm64, since the arm64 fault code is derived from arm.

Quoted 16 lines

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Utilizing FPE_FIXME, BUS_FIXME, TRAP_FIXME siginfo_layout will now return SIL_FAULT and the appropriate fields will be reliably copied.

But folks this is a new and unique kind of bad. This is massively

untested code bad.  This is inventing new and unique was to get
siginfo wrong bad.  This is don't even think about Posix or what
siginfo means bad.  This is lots of eyeballs all missing the fact

that the code does the wrong thing bad. This is getting stuck and keep making the same mistake bad.

Quoted 6 lines

I really hope we can find a non userspace breaking fix for this on a port as new as arm64.

Possible ABI fixes include:
- Send the signal without siginfo
- Don't generate a signal

The above two sould like ABI breaks?

Quoted 2 lines

- Possibly assign and use an appropriate si_code
- Don't handle cases which can't happen

I think a mixture of these two is the best approach.

In any case, si_code == 0 here doesn't seem to have any explicit meaning. I think we can translate all of the arm64 faults to proper si_codes -- see my sketch below. Probably means a bit more thought though.

The only counterargument would be if there is software relying on these bogus signal cases getting si_code == 0 for a useful purpose.

The main reason I see to check for SI_USER is to allow a process to filter out spurious signals (say, an asynchronous I/O signal for which si_value would be garbage), and to print out diagnostics before (in the case of a well-behaved program) resetting the signal to SIG_DFL and killing itself to report the signal to the waiter.

Daemons may be more discerning about who is allowed to signal them, but overloading SIGBUS (say) as an IPC channel sounds like a very odd thing to do. The same probably applies to any signal that has nontrivial metadata.

Have you found software that is impacted by this in practice?

[...]

arch/arm64/kernel/fpsimd.c asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) · line 873

Quoted 10 lines

+++ b/arch/arm64/kernel/fpsimd.c

arch/arm64/kernel/fpsimd.c +1 -1

@@ -867,7 +867,7 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs)
 asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs)
 {
 	siginfo_t info;
-	unsigned int si_code = 0;
+	unsigned int si_code = FPE_FIXME;
 
 	if (esr & FPEXC_IOF)
 		si_code = FPE_FLTINV;

This 0 can happen for vector operations where the implementation may not be able to report exactly what happened, for example where the implementer didn't want to pay the cost of tracking exactly what went wrong in each lane.

However, the FPEXC_* bits can be garbage in such a case rather than being all zero: we should be checking the TFV bit in the ESR here. This may be a bug.

Perhaps FPE_FLTINV should be returned in si_code for such cases: it's not otherwise used on arm64 -- invalid instructions would be reported as SIGILL/ILL_ILLOPC instead).

Otherwise, we might want to define a new code or arbitrarily pick one of the existing FLT_* since this is really a more benign condition than executing an illegal instruction. Alternatively, treat the fault as spurious and suppress it, but that doesn't feel right either.

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 599

Quoted 10 lines

arch/arm64/mm/fault.c +1 -1

index 9b7f89df49db..abe200587334 100644
@@ -596,7 +596,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 
 	info.si_signo = SIGBUS;
 	info.si_errno = 0;
-	info.si_code  = 0;
+	info.si_code  = BUS_FIXME;

Probably BUS_OBJERR.

Quoted 5 lines

arch/arm64/mm/fault.c +0 -0

 	if (esr & ESR_ELx_FnV)
 		info.si_addr = NULL;
 	else

@@ -607,70 +607,70 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 }

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 613

Quoted 10 lines

arch/arm64/mm/fault.c +4 -4

 
 static const struct fault_info fault_info[] = {
-	{ do_bad,		SIGBUS,  0,		"ttbr address size fault"	},
-	{ do_bad,		SIGBUS,  0,		"level 1 address size fault"	},
-	{ do_bad,		SIGBUS,  0,		"level 2 address size fault"	},
-	{ do_bad,		SIGBUS,  0,		"level 3 address size fault"	},
+	{ do_bad,		SIGBUS,  BUS_FIXME,	"ttbr address size fault"	},
+	{ do_bad,		SIGBUS,  BUS_FIXME,	"level 1 address size fault"	},
+	{ do_bad,		SIGBUS,  BUS_FIXME,	"level 2 address size fault"	},
+	{ do_bad,		SIGBUS,  BUS_FIXME,	"level 3 address size fault"	},

Pagetable screwup or kernel/system/CPU bug -> SIGKILL, or panic().

[...]

Quoted 2 lines

arch/arm64/mm/fault.c +1 -1

-	{ do_bad,		SIGBUS,  0,		"unknown 8"			},
+	{ do_bad,		SIGBUS,  BUS_FIXME,	"unknown 8"			},

[...]

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 622

Quoted 1 line

arch/arm64/mm/fault.c +1 -0

+	{ do_bad,		SIGBUS,  BUS_FIXME,	"unknown 12"			},

Not architected, so they could mean absolutely anything. If they can happen at all, they are probably unsafe to ignore.

-> SIGKILL, or panic().

Similary for all the "unknown" codes in the table, which I omit for brevity.

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 626

Quoted 1 line

arch/arm64/mm/fault.c +1 -0

+	{ do_sea,		SIGBUS,  BUS_FIXME,	"synchronous external abort"	},

This si_code seems to be a fallback for if ACPI is absent or doesn't know what to do with this error.

-> SIGBUS/BUS_OBJERR?

Can probably legitimately happen for userspace for suitable MMIO mappings.

Perhaps it's more serious though in the presence of ACPI. Do we expect that ACPI can diagnose all localisable errors?

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 633

Quoted 4 lines

arch/arm64/mm/fault.c +4 -0

+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 0 (translation table walk)"	},
+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 1 (translation table walk)"	},
+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 2 (translation table walk)"	},
+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 3 (translation table walk)"	},

Pagetable screwup or kernel/system/CPU bug -> SIGKILL, or panic().

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 634

Quoted 1 line

arch/arm64/mm/fault.c +1 -0

+	{ do_sea,		SIGBUS,  BUS_FIXME,	"synchronous parity or ECC error" },	// Reserved when RAS is implemented

Possibly SIGBUS/BUS_MCEERR_AR (though I don't know exactly what userspace is supposed to do with this or whether this implies the existence or certain kernel features for managing the error that may not be present on arm64...)

Otherwise, SIGKILL.

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 641

Quoted 4 lines

arch/arm64/mm/fault.c +4 -0

+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 0 synchronous parity error (translation table walk)"	},	// Reserved when RAS is implemented
+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 1 synchronous parity error (translation table walk)"	},	// Reserved when RAS is implemented
+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 2 synchronous parity error (translation table walk)"	},	// Reserved when RAS is implemented
+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 3 synchronous parity error (translation table walk)"	},	// Reserved when RAS is implemented

Process page tables corrupt: if the kernel couldn't fix this, the process can't reasonably fix it -> SIGKILL

Since this is a RAS-type error it could be triggered by a cosmic ray rather than requiring a kernel or system bug or other major failure, so we probably shouldn't panic the system if the error is localisable to a particular process.

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 658

Quoted 2 lines

arch/arm64/mm/fault.c +1 -0

 	{ do_alignment_fault,	SIGBUS,  BUS_ADRALN,	"alignment fault"		},
+	{ do_bad,		SIGBUS,  BUS_FIXME,	"TLB conflict abort"		},

Broken kernel, kernel memory corruption, CPU/system bug etc.:
SIGKILL or panic().

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 659

Quoted 1 line

arch/arm64/mm/fault.c +1 -0

+	{ do_bad,		SIGBUS,  BUS_FIXME,	"Unsupported atomic hardware update fault"	},

Broken kernel, kernel memory corruption, CPU/system bug etc.:
SIGKILL or panic().

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 662

Quoted 1 line

arch/arm64/mm/fault.c +1 -0

+	{ do_bad,		SIGBUS,  BUS_FIXME,	"implementation fault (lockdown abort)" },

Userspace shouldn't have access to lockdown: kernel/system bug -> SIGKILL or panic().

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 663

Quoted 1 line

arch/arm64/mm/fault.c +1 -0

+	{ do_bad,		SIGBUS,  BUS_FIXME,	"implementation fault (unsupported exclusive)" },

If running on an implementation where this fault can happen in response to an exclusive load/store issued by userspace may fail somewhere in the memory system, this should probably be SIGBUS/BUS_OBJERR (or possibly a new BUS_* code).

This one may need to be hardware-dependent, if this fault can mean something different depending on the hardware (I'm gussing this possibility from "implementation" -- I've not checked the docs.)

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 672

Quoted 2 lines

arch/arm64/mm/fault.c +2 -0

+	{ do_bad,		SIGBUS,  BUS_FIXME,	"section domain fault"		},
+	{ do_bad,		SIGBUS,  BUS_FIXME,	"page domain fault"		},

Broken kernel, kernel memory corruption, CPU/system bug etc.:
SIGKILL or panic().

Quoted 2 lines

};

arch/arm64/mm/fault.c static struct fault_info __refdata debug_fault_info[] = { · line 742

Quoted 6 lines

int handle_guest_sea(phys_addr_t addr, unsigned int esr)

arch/arm64/mm/fault.c +3 -0

@@ -739,11 +739,11 @@ static struct fault_info __refdata debug_fault_info[] = {
+	{ do_bad,	SIGBUS,		BUS_FIXME,	"unknown 3"		},
+	{ do_bad,	SIGTRAP,	TRAP_FIXME,	"aarch32 vector catch"	},
+	{ do_bad,	SIGBUS,		BUS_FIXME,	"unknown 7"		},
 };

Impossible (?), or meaning unknown.
SIGKILL/panic() for these? Or possibly (since these are probably well localised errors) SIGILL/ILL_ILLOPC.

Quoted 18 lines · Dave Martin

Quoted 4 lines · Eric W. Biederman

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

I think this situation may have come about because 0 is used as a padding value for "impossible" cases -- i.e., things that can't happen unless the kernel is broken, or things that are too unrecoverable for clean error reporting to be helpful.

In general, I think these values are not expected to reach userspace in practice.

This is not an excuse though -- and not 100% true -- so it's certainly worthy of cleanup.

Quoted 2 lines

It would be good to approach this similarly for arm and arm64, since the arm64 fault code is derived from arm.

In this case the fault_info is something I have only seen on arm64. I have been approaching all architectures the same way.

If there is insufficient information without architecture expertise to fix this class of error I have been ading FPE_FIXME to them.

Quoted 16 lines

Quoted 16 lines

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Utilizing FPE_FIXME, BUS_FIXME, TRAP_FIXME siginfo_layout will now return SIL_FAULT and the appropriate fields will be reliably copied.

But folks this is a new and unique kind of bad. This is massively

untested code bad.  This is inventing new and unique was to get
siginfo wrong bad.  This is don't even think about Posix or what
siginfo means bad.  This is lots of eyeballs all missing the fact

that the code does the wrong thing bad. This is getting stuck and keep making the same mistake bad.

Quoted 8 lines

Quoted 6 lines

I really hope we can find a non userspace breaking fix for this on a port as new as arm64.

Possible ABI fixes include:
- Send the signal without siginfo
- Don't generate a signal

The above two sould like ABI breaks?

They are ways I have seen code on other platforms deal with not information to generate siginfo. Sending the signal without siginfo is roughly equivalent to your send SIGKILL suggestion below.

A good example of that is code that calls force_sigsegv.

Calling "force_sig(SIGBUS, current);" is perfectly valid. And then the parent when it reaped the process would have a little more information to go on when guessing what happened to the process.

Quoted 8 lines

Quoted 2 lines

- Possibly assign and use an appropriate si_code
- Don't handle cases which can't happen

I think a mixture of these two is the best approach.

In any case, si_code == 0 here doesn't seem to have any explicit meaning. I think we can translate all of the arm64 faults to proper si_codes -- see my sketch below. Probably means a bit more thought though.

Yes I would be very happy to see that.

Quoted 9 lines

The only counterargument would be if there is software relying on these bogus signal cases getting si_code == 0 for a useful purpose.

The main reason I see to check for SI_USER is to allow a process to filter out spurious signals (say, an asynchronous I/O signal for which si_value would be garbage), and to print out diagnostics before (in the case of a well-behaved program) resetting the signal to SIG_DFL and killing itself to report the signal to the waiter.

Quoted 4 lines

Daemons may be more discerning about who is allowed to signal them, but overloading SIGBUS (say) as an IPC channel sounds like a very odd thing to do. The same probably applies to any signal that has nontrivial metadata.

Agreed. Although I have seen ltp test cases that do crazy things like that.

Quoted 1 line

Have you found software that is impacted by this in practice?

No.

I don't expect many userspace applications look at siginfo and everything I have found is some rare hard to trigger non-x86 case which limits the exposure to userspace applications tremendously.

The angle I am coming at all of this from is that the linux kernel code that filled out out struct siginfo was not comprehensible or correct.

Internal to the kernel it was using a magic value (not exportable to userspace) in the upper bits of si_code. That was causing problems for signal injection and converting signals from 32bit to 64bit, and from 64bit to 32bit.

So I wrote kernel/signal.c:siginfo_layout() to figure out which fields of struct siginfo should be sent to userspace. In doing so I discovered that using 0 in si_code (aka SI_USER) is ambiguous, and problematic.

Unfortuantely in most of the cases I have spotted using 0 in the si_code requires architectural knowledge that I don't currently have to sort out. So the best I can do is change si_code from 0 to FPE_FIXME/BUS_FIXME/TRAP_FIXME and bring the architecture maintainers attention to this area.

One of the problems that results from all of this is that we copy
unitialized data to userspace. I am slowly unifying and cleaning the
code up so that the code is simple enough we can be certain we are not copying unitialized data to userspace.

With si_coes of FPE_FIXME/BUS_FIXME/TRAP_FIXME I can at least attempt to keep the craziness from happening.

My next step is to unify struct siginfo and struct compat_siginfo and the functions that copy them to userspace because there are very siginficant problems there.

All of that said I like the way you are thinking about fixing these issues.

Quoted 20 lines

arch/arm64/kernel/fpsimd.c asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) · line 873

Quoted 10 lines

+++ b/arch/arm64/kernel/fpsimd.c

arch/arm64/kernel/fpsimd.c +1 -1

@@ -867,7 +867,7 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs)
 asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs)
 {
 	siginfo_t info;
-	unsigned int si_code = 0;
+	unsigned int si_code = FPE_FIXME;
 
 	if (esr & FPEXC_IOF)
 		si_code = FPE_FLTINV;

This 0 can happen for vector operations where the implementation may not be able to report exactly what happened, for example where the implementer didn't want to pay the cost of tracking exactly what went wrong in each lane.

However, the FPEXC_* bits can be garbage in such a case rather than being all zero: we should be checking the TFV bit in the ESR here. This may be a bug.

Quoted 8 lines

Perhaps FPE_FLTINV should be returned in si_code for such cases: it's not otherwise used on arm64 -- invalid instructions would be reported as SIGILL/ILL_ILLOPC instead).

Otherwise, we might want to define a new code or arbitrarily pick one of the existing FLT_* since this is really a more benign condition than executing an illegal instruction. Alternatively, treat the fault as spurious and suppress it, but that doesn't feel right either.

I would love to see this sorted out. There is a very similar pattern on several different architectures. I suspect if we have a clean solution on one architecture the other architectures will be able to use that solution as well.

Quoted 20 lines · Eric W. Biederman

Quoted 18 lines · Dave Martin

Quoted 4 lines · Eric W. Biederman

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

I think this situation may have come about because 0 is used as a padding value for "impossible" cases -- i.e., things that can't happen unless the kernel is broken, or things that are too unrecoverable for clean error reporting to be helpful.

In general, I think these values are not expected to reach userspace in practice.

This is not an excuse though -- and not 100% true -- so it's certainly worthy of cleanup.

Quoted 5 lines

Quoted 2 lines

It would be good to approach this similarly for arm and arm64, since the arm64 fault code is derived from arm.

In this case the fault_info is something I have only seen on arm64. I have been approaching all architectures the same way.

Bad guess on my part; this table-driven approach seems to be new for arm64.

Quoted 2 lines

If there is insufficient information without architecture expertise to fix this class of error I have been ading FPE_FIXME to them.

Fair enough.

Quoted 61 lines

Quoted 16 lines

Quoted 16 lines

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Utilizing FPE_FIXME, BUS_FIXME, TRAP_FIXME siginfo_layout will now return SIL_FAULT and the appropriate fields will be reliably copied.

But folks this is a new and unique kind of bad. This is massively

untested code bad.  This is inventing new and unique was to get
siginfo wrong bad.  This is don't even think about Posix or what
siginfo means bad.  This is lots of eyeballs all missing the fact

that the code does the wrong thing bad. This is getting stuck and keep making the same mistake bad.

Quoted 8 lines

Quoted 6 lines

I really hope we can find a non userspace breaking fix for this on a port as new as arm64.

Possible ABI fixes include:
- Send the signal without siginfo
- Don't generate a signal

The above two sould like ABI breaks?

They are ways I have seen code on other platforms deal with not information to generate siginfo. Sending the signal without siginfo is roughly equivalent to your send SIGKILL suggestion below.

A good example of that is code that calls force_sigsegv.

Calling "force_sig(SIGBUS, current);" is perfectly valid. And then the parent when it reaped the process would have a little more information to go on when guessing what happened to the process.

Quoted 8 lines

Quoted 2 lines

- Possibly assign and use an appropriate si_code
- Don't handle cases which can't happen

I think a mixture of these two is the best approach.

In any case, si_code == 0 here doesn't seem to have any explicit meaning. I think we can translate all of the arm64 faults to proper si_codes -- see my sketch below. Probably means a bit more thought though.

Yes I would be very happy to see that.

Quoted 9 lines

The only counterargument would be if there is software relying on these bogus signal cases getting si_code == 0 for a useful purpose.

The main reason I see to check for SI_USER is to allow a process to filter out spurious signals (say, an asynchronous I/O signal for which si_value would be garbage), and to print out diagnostics before (in the case of a well-behaved program) resetting the signal to SIG_DFL and killing itself to report the signal to the waiter.

Quoted 4 lines

Daemons may be more discerning about who is allowed to signal them, but overloading SIGBUS (say) as an IPC channel sounds like a very odd thing to do. The same probably applies to any signal that has nontrivial metadata.

Quoted 6 lines

Agreed. Although I have seen ltp test cases that do crazy things like that.

Quoted 1 line

Have you found software that is impacted by this in practice?

No.

Searching for si_code on codesearch.debian.net, I looked at a few random hits. Although this is far from exhaustive, I saw no instance of code that assumes some arch-specific meaning for SI_USER (or 0).

Most code seems to check for signal-specific si_code values before assuming that signal-specific signifo fields are valid; or for SI_USER (or si_code <= 0) before assuming that si_uid and si_pid are valid.

Returning proper values for si_code values in place of "0" would fix rather than break such cases.

Quoted 6 lines

I don't expect many userspace applications look at siginfo and everything I have found is some rare hard to trigger non-x86 case which limits the exposure to userspace applications tremendously.

The angle I am coming at all of this from is that the linux kernel code that filled out out struct siginfo was not comprehensible or correct.

I think "not comprehensible or correct" is a pretty good summary of all signal-related code...

Quoted 20 lines

Internal to the kernel it was using a magic value (not exportable to userspace) in the upper bits of si_code. That was causing problems for signal injection and converting signals from 32bit to 64bit, and from 64bit to 32bit.

So I wrote kernel/signal.c:siginfo_layout() to figure out which fields of struct siginfo should be sent to userspace. In doing so I discovered that using 0 in si_code (aka SI_USER) is ambiguous, and problematic.

Unfortuantely in most of the cases I have spotted using 0 in the si_code requires architectural knowledge that I don't currently have to sort out. So the best I can do is change si_code from 0 to FPE_FIXME/BUS_FIXME/TRAP_FIXME and bring the architecture maintainers attention to this area.

One of the problems that results from all of this is that we copy
unitialized data to userspace. I am slowly unifying and cleaning the
code up so that the code is simple enough we can be certain we are not copying unitialized data to userspace.

Quoted 6 lines

With si_coes of FPE_FIXME/BUS_FIXME/TRAP_FIXME I can at least attempt to keep the craziness from happening.

My next step is to unify struct siginfo and struct compat_siginfo and the functions that copy them to userspace because there are very siginficant problems there.

All of which sounds valuable.

Quoted 2 lines

All of that said I like the way you are thinking about fixing these issues.

Is it feasible to have a different internal constant for SI_USER? Then the generic could warn if it sees si_code == 0. If the special nonzero KERNEL_SI_USER is seen instead, it is silently translated to SI_USER (0) for userspace. This might help us track down cases where 0 is passed by accident.

It may not be worth it though: if the affected cases are ones that happen never or almost never, a runtime BUG_ON() may not be helpful for tracking them down.

Also, I'm making an assumption that si_code always flows through some generic code before reaching userspace (maybe untrue).

Quoted 29 lines

Quoted 20 lines

arch/arm64/kernel/fpsimd.c asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) · line 873

Quoted 10 lines

+++ b/arch/arm64/kernel/fpsimd.c

arch/arm64/kernel/fpsimd.c +1 -1

@@ -867,7 +867,7 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs)
 asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs)
 {
 	siginfo_t info;
-	unsigned int si_code = 0;
+	unsigned int si_code = FPE_FIXME;
 
 	if (esr & FPEXC_IOF)
 		si_code = FPE_FLTINV;

This 0 can happen for vector operations where the implementation may not be able to report exactly what happened, for example where the implementer didn't want to pay the cost of tracking exactly what went wrong in each lane.

However, the FPEXC_* bits can be garbage in such a case rather than being all zero: we should be checking the TFV bit in the ESR here. This may be a bug.

Quoted 8 lines

Perhaps FPE_FLTINV should be returned in si_code for such cases: it's not otherwise used on arm64 -- invalid instructions would be reported as SIGILL/ILL_ILLOPC instead).

Otherwise, we might want to define a new code or arbitrarily pick one of the existing FLT_* since this is really a more benign condition than executing an illegal instruction. Alternatively, treat the fault as spurious and suppress it, but that doesn't feel right either.

Quoted 4 lines

I would love to see this sorted out. There is a very similar pattern on several different architectures. I suspect if we have a clean solution on one architecture the other architectures will be able to use that solution as well.

Since user code that relies on checking si_code for fp exceptions will probably already break in these cases, we can probably fudge things a bit here.

I'll have a think. IEEE may also define some rules that are relevant here...

For the proposed conversion of the si_code==0 cases for arm64, I'll draft an RFC for discussion (hopefully sometime this week).

Quoted 164 lines · Dave Martin

Quoted 20 lines · Eric W. Biederman

Quoted 18 lines · Dave Martin

Quoted 4 lines · Eric W. Biederman

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

I think this situation may have come about because 0 is used as a padding value for "impossible" cases -- i.e., things that can't happen unless the kernel is broken, or things that are too unrecoverable for clean error reporting to be helpful.

In general, I think these values are not expected to reach userspace in practice.

This is not an excuse though -- and not 100% true -- so it's certainly worthy of cleanup.

Quoted 5 lines

Quoted 2 lines

It would be good to approach this similarly for arm and arm64, since the arm64 fault code is derived from arm.

In this case the fault_info is something I have only seen on arm64. I have been approaching all architectures the same way.

Bad guess on my part; this table-driven approach seems to be new for arm64.

Quoted 2 lines

If there is insufficient information without architecture expertise to fix this class of error I have been ading FPE_FIXME to them.

Fair enough.

Quoted 61 lines

Quoted 16 lines

Quoted 16 lines

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Utilizing FPE_FIXME, BUS_FIXME, TRAP_FIXME siginfo_layout will now return SIL_FAULT and the appropriate fields will be reliably copied.

But folks this is a new and unique kind of bad. This is massively

untested code bad.  This is inventing new and unique was to get
siginfo wrong bad.  This is don't even think about Posix or what
siginfo means bad.  This is lots of eyeballs all missing the fact

that the code does the wrong thing bad. This is getting stuck and keep making the same mistake bad.

Quoted 8 lines

Quoted 6 lines

I really hope we can find a non userspace breaking fix for this on a port as new as arm64.

Possible ABI fixes include:
- Send the signal without siginfo
- Don't generate a signal

The above two sould like ABI breaks?

They are ways I have seen code on other platforms deal with not information to generate siginfo. Sending the signal without siginfo is roughly equivalent to your send SIGKILL suggestion below.

A good example of that is code that calls force_sigsegv.

Calling "force_sig(SIGBUS, current);" is perfectly valid. And then the parent when it reaped the process would have a little more information to go on when guessing what happened to the process.

Quoted 8 lines

Quoted 2 lines

- Possibly assign and use an appropriate si_code
- Don't handle cases which can't happen

I think a mixture of these two is the best approach.

In any case, si_code == 0 here doesn't seem to have any explicit meaning. I think we can translate all of the arm64 faults to proper si_codes -- see my sketch below. Probably means a bit more thought though.

Yes I would be very happy to see that.

Quoted 9 lines

The only counterargument would be if there is software relying on these bogus signal cases getting si_code == 0 for a useful purpose.

The main reason I see to check for SI_USER is to allow a process to filter out spurious signals (say, an asynchronous I/O signal for which si_value would be garbage), and to print out diagnostics before (in the case of a well-behaved program) resetting the signal to SIG_DFL and killing itself to report the signal to the waiter.

Quoted 4 lines

Daemons may be more discerning about who is allowed to signal them, but overloading SIGBUS (say) as an IPC channel sounds like a very odd thing to do. The same probably applies to any signal that has nontrivial metadata.

Quoted 6 lines

Agreed. Although I have seen ltp test cases that do crazy things like that.

Quoted 1 line

Have you found software that is impacted by this in practice?

No.

Searching for si_code on codesearch.debian.net, I looked at a few random hits. Although this is far from exhaustive, I saw no instance of code that assumes some arch-specific meaning for SI_USER (or 0).

Most code seems to check for signal-specific si_code values before assuming that signal-specific signifo fields are valid; or for SI_USER (or si_code <= 0) before assuming that si_uid and si_pid are valid.

Returning proper values for si_code values in place of "0" would fix rather than break such cases.

Quoted 6 lines

I don't expect many userspace applications look at siginfo and everything I have found is some rare hard to trigger non-x86 case which limits the exposure to userspace applications tremendously.

The angle I am coming at all of this from is that the linux kernel code that filled out out struct siginfo was not comprehensible or correct.

I think "not comprehensible or correct" is a pretty good summary of all signal-related code...

Quoted 20 lines

Internal to the kernel it was using a magic value (not exportable to userspace) in the upper bits of si_code. That was causing problems for signal injection and converting signals from 32bit to 64bit, and from 64bit to 32bit.

So I wrote kernel/signal.c:siginfo_layout() to figure out which fields of struct siginfo should be sent to userspace. In doing so I discovered that using 0 in si_code (aka SI_USER) is ambiguous, and problematic.

Unfortuantely in most of the cases I have spotted using 0 in the si_code requires architectural knowledge that I don't currently have to sort out. So the best I can do is change si_code from 0 to FPE_FIXME/BUS_FIXME/TRAP_FIXME and bring the architecture maintainers attention to this area.

One of the problems that results from all of this is that we copy
unitialized data to userspace. I am slowly unifying and cleaning the
code up so that the code is simple enough we can be certain we are not copying unitialized data to userspace.

Quoted 6 lines

With si_coes of FPE_FIXME/BUS_FIXME/TRAP_FIXME I can at least attempt to keep the craziness from happening.

My next step is to unify struct siginfo and struct compat_siginfo and the functions that copy them to userspace because there are very siginficant problems there.

All of which sounds valuable.

Quoted 2 lines

All of that said I like the way you are thinking about fixing these issues.

Is it feasible to have a different internal constant for SI_USER? Then the generic could warn if it sees si_code == 0. If the special nonzero KERNEL_SI_USER is seen instead, it is silently translated to SI_USER (0) for userspace. This might help us track down cases where 0 is passed by accident.

Quoted 6 lines

It may not be worth it though: if the affected cases are ones that happen never or almost never, a runtime BUG_ON() may not be helpful for tracking them down.

Also, I'm making an assumption that si_code always flows through some generic code before reaching userspace (maybe untrue).

The code does flow through a generic path, and I am in the middle of tightening that up right now. As filling out siginfo is error prone, and I need a guarantee that all of struct siginfo is initialized. Adding a warning if a arch fault hander uses si_code == 0 aka SI_USER would not be hard.

Given what you have found. Given that it seems to match my experience we can almost certainly change the code to just warn when the 0 is passed in for the si_code for fault handlers.

I want to ensure that all of the fields are filled in before I do that or else I risk passing unitialized values to userspace. But I like that as the long term goal for this code.

Quoted 38 lines

Quoted 29 lines

Quoted 20 lines

Review asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) · line 873

Quoted 10 lines

+++ b/arch/arm64/kernel/fpsimd.c

(quoted diff) +1 -1

@@ -867,7 +867,7 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs)
 asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs)
 {
 	siginfo_t info;
-	unsigned int si_code = 0;
+	unsigned int si_code = FPE_FIXME;
 
 	if (esr & FPEXC_IOF)
 		si_code = FPE_FLTINV;

This 0 can happen for vector operations where the implementation may not be able to report exactly what happened, for example where the implementer didn't want to pay the cost of tracking exactly what went wrong in each lane.

However, the FPEXC_* bits can be garbage in such a case rather than being all zero: we should be checking the TFV bit in the ESR here. This may be a bug.

Quoted 8 lines

Perhaps FPE_FLTINV should be returned in si_code for such cases: it's not otherwise used on arm64 -- invalid instructions would be reported as SIGILL/ILL_ILLOPC instead).

Otherwise, we might want to define a new code or arbitrarily pick one of the existing FLT_* since this is really a more benign condition than executing an illegal instruction. Alternatively, treat the fault as spurious and suppress it, but that doesn't feel right either.

Quoted 4 lines

I would love to see this sorted out. There is a very similar pattern on several different architectures. I suspect if we have a clean solution on one architecture the other architectures will be able to use that solution as well.

Since user code that relies on checking si_code for fp exceptions will probably already break in these cases, we can probably fudge things a bit here.

Quoted 5 lines

I'll have a think. IEEE may also define some rules that are relevant here...

For the proposed conversion of the si_code==0 cases for arm64, I'll draft an RFC for discussion (hopefully sometime this week).

Sounds good.

I will keep FPE_FIXME as a place holder until this gets sorted out.

There is a second issue I am looking at in this location, and maybe I don't have to address it now. But it looks like the code is calling send_sig_info instead of force_sig_info for a synchronous exception. Am I reading that correctly?

I will keep FPE_FIXME as a place holder until this gets sorted out.

There is a second issue I am looking at in this location, and maybe I don't have to address it now. But it looks like the code is calling send_sig_info instead of force_sig_info for a synchronous exception. Am I reading that correctly?

Quoted 6 lines · Eric W. Biederman

I will keep FPE_FIXME as a place holder until this gets sorted out.

There is a second issue I am looking at in this location, and maybe I don't have to address it now. But it looks like the code is calling send_sig_info instead of force_sig_info for a synchronous exception. Am I reading that correctly?

VFP used to use force_sig_info(), but it seems to be really the wrong call to use. force_sig_info() checks whether the program decided to ignore or block the signal, and if it did, replaces the signal handler with the default handler and unblocks the signal.

Are you really suggesting that FP all FP signals should get this treatment?

Quoted 8 lines · Russell King - ARM Linux

Quoted 6 lines · Eric W. Biederman

I will keep FPE_FIXME as a place holder until this gets sorted out.

There is a second issue I am looking at in this location, and maybe I don't have to address it now. But it looks like the code is calling send_sig_info instead of force_sig_info for a synchronous exception. Am I reading that correctly?

Quoted 7 lines

VFP used to use force_sig_info(), but it seems to be really the wrong call to use. force_sig_info() checks whether the program decided to ignore or block the signal, and if it did, replaces the signal handler with the default handler and unblocks the signal.

Are you really suggesting that FP all FP signals should get this treatment?

feenableexcept(FE_OVERFLOW) kind of means "I can't run safely past an fp overflow exception, please signal me instead".

If the process also blocked SIGFPE, that could be taken to mean "I can't run safely past an fp overflow exception _and_ I can't take SIGFPE either" ... i.e., if an fp overflow happens there is no way to proceed and it's really fatal.

What SIG_IGN ought to mean is rather more debatable, but again, the process could be asking for two opposite things: guarantee a SIGFPE is delivered instead of running past an fp exception, and also guarantee that SIGFPE is _not_ delivered.

It looks like arm and arm64 are different from most other arches (including x86) here, but I'm not sure what is considered correct, and it looks like the answer is not standardised. There's a possibility that some software goes subtly wrong on arm/arm64 where on other arches it would get terminated with SIGKILL.

Whether this matters depends on how harmless the fp exception is to the work of the program. I think if an exception is set to trap via feenableexcept() then that's a strong hint the programmer thinks that exception is not harmless. OTOH, trapping is not always available anyway...

Was there some particular program being broken by the force_sig_info() here?

Quoted 1 line · Dave Martin

On Wed, Jan 17, 2018 at 11:57:09AM +0000, Russell King - ARM Linux wrote:

Quoted 27 lines

Quoted 7 lines

VFP used to use force_sig_info(), but it seems to be really the wrong call to use. force_sig_info() checks whether the program decided to ignore or block the signal, and if it did, replaces the signal handler with the default handler and unblocks the signal.

Are you really suggesting that FP all FP signals should get this treatment?

feenableexcept(FE_OVERFLOW) kind of means "I can't run safely past an fp overflow exception, please signal me instead".

If the process also blocked SIGFPE, that could be taken to mean "I can't run safely past an fp overflow exception _and_ I can't take SIGFPE either" ... i.e., if an fp overflow happens there is no way to proceed and it's really fatal.

What SIG_IGN ought to mean is rather more debatable, but again, the process could be asking for two opposite things: guarantee a SIGFPE is delivered instead of running past an fp exception, and also guarantee that SIGFPE is _not_ delivered.

It looks like arm and arm64 are different from most other arches (including x86) here, but I'm not sure what is considered correct, and it looks like the answer is not standardised. There's a possibility that some software goes subtly wrong on arm/arm64 where on other arches it would get terminated with SIGKILL.

Quoted 5 lines

Whether this matters depends on how harmless the fp exception is to the work of the program. I think if an exception is set to trap via feenableexcept() then that's a strong hint the programmer thinks that exception is not harmless. OTOH, trapping is not always available anyway...

Like many of these things, there is no clear answer. It's a set of conflicting requirements, and as you point out, even if you've called feenableexcept(), you are not guaranteed to get a trap.

However, do remember that FP exceptions on ARM hardware are already asynchronous - they get reported by the _next_ FP operation to the one that caused them, which means they could be raised by a library function sometime after it occured (when the library function decides to save the FP registers to the stack before it makes use of them.) It's entirely possible that the library function has blocked FP signals temporarily (not explicitly, just decided to block all signals while it does something sensitive) and will unblock them again afterwards - at which point we get the SIGFPE, and it would be quite right to deliver that signal to the user SIGFPE handler, rather than forcing it onto the program mid-library function.

It's also possible that SIGFPE could be blocked by another signal handler having been invoked, and it triggers the latent generation of the SIGFPE.

I'd be more inclined to agree with you if VFP exceptions were synchronous but they aren't.

Quoted 2 lines · Russell King - ARM Linux

Quoted 1 line · Dave Martin

On Wed, Jan 17, 2018 at 11:57:09AM +0000, Russell King - ARM Linux wrote:

[...]

Quoted 49 lines

Quoted 27 lines

Quoted 7 lines

VFP used to use force_sig_info(), but it seems to be really the wrong call to use. force_sig_info() checks whether the program decided to ignore or block the signal, and if it did, replaces the signal handler with the default handler and unblocks the signal.

Are you really suggesting that FP all FP signals should get this treatment?

feenableexcept(FE_OVERFLOW) kind of means "I can't run safely past an fp overflow exception, please signal me instead".

If the process also blocked SIGFPE, that could be taken to mean "I can't run safely past an fp overflow exception _and_ I can't take SIGFPE either" ... i.e., if an fp overflow happens there is no way to proceed and it's really fatal.

What SIG_IGN ought to mean is rather more debatable, but again, the process could be asking for two opposite things: guarantee a SIGFPE is delivered instead of running past an fp exception, and also guarantee that SIGFPE is _not_ delivered.

It looks like arm and arm64 are different from most other arches (including x86) here, but I'm not sure what is considered correct, and it looks like the answer is not standardised. There's a possibility that some software goes subtly wrong on arm/arm64 where on other arches it would get terminated with SIGKILL.

Quoted 5 lines

Whether this matters depends on how harmless the fp exception is to the work of the program. I think if an exception is set to trap via feenableexcept() then that's a strong hint the programmer thinks that exception is not harmless. OTOH, trapping is not always available anyway...

Like many of these things, there is no clear answer. It's a set of conflicting requirements, and as you point out, even if you've called feenableexcept(), you are not guaranteed to get a trap.

However, do remember that FP exceptions on ARM hardware are already asynchronous - they get reported by the _next_ FP operation to the one that caused them, which means they could be raised by a library function sometime after it occured (when the library function decides to save the FP registers to the stack before it makes use of them.) It's entirely possible that the library function has blocked FP signals temporarily (not explicitly, just decided to block all signals while it does something sensitive) and will unblock them again afterwards - at which point we get the SIGFPE, and it would be quite right to deliver that signal to the user SIGFPE handler, rather than forcing it onto the program mid-library function.

Quoted 5 lines

It's also possible that SIGFPE could be blocked by another signal handler having been invoked, and it triggers the latent generation of the SIGFPE.

I'd be more inclined to agree with you if VFP exceptions were synchronous but they aren't.

Hmmm, it looks like imprecise fp exception traps are disallowed from ARMv8 onwards. I guess they made more sense when the FPU really was a coprocessor, or at least semidetached from the integer core.

I think force_sig_info() makes sense here if and only if the traps are guaranteed to be precise, so we probably should use this on arm64. Not arm though (alpha doesn't either, if I understand the code correctly.)

Does that make sense?

Apparently, few recent cores (at least ARM's own ones) support fp exception trapping anyway... 1176 may be the most recent.

Quoted 1 line · Dave Martin

On Wed, Jan 17, 2018 at 12:37:52PM +0000, Russell King - ARM Linux wrote:

Quoted 14 lines

Quoted 2 lines

I'd be more inclined to agree with you if VFP exceptions were synchronous but they aren't.

Hmmm, it looks like imprecise fp exception traps are disallowed from ARMv8 onwards. I guess they made more sense when the FPU really was a coprocessor, or at least semidetached from the integer core.

I think force_sig_info() makes sense here if and only if the traps are guaranteed to be precise, so we probably should use this on arm64. Not arm though (alpha doesn't either, if I understand the code correctly.)

Does that make sense?

Quoted 2 lines

Apparently, few recent cores (at least ARM's own ones) support fp exception trapping anyway... 1176 may be the most recent.

... and that makes the feenableexcept() argument about "A program really wants to know" moot. It can enable the exceptions in the FPSCR but its never going to receive a SIGFPE on CPUs that don't do exception trapping.

Quoted 9 lines · Dave Martin

Quoted 8 lines · Russell King - ARM Linux

Quoted 6 lines · Eric W. Biederman

I will keep FPE_FIXME as a place holder until this gets sorted out.

There is a second issue I am looking at in this location, and maybe I don't have to address it now. But it looks like the code is calling send_sig_info instead of force_sig_info for a synchronous exception. Am I reading that correctly?

Quoted 4 lines

Quoted 4 lines

VFP used to use force_sig_info(), but it seems to be really the wrong call to use. force_sig_info() checks whether the program decided to ignore or block the signal, and if it did, replaces the signal handler with the default handler and unblocks the signal.

Quoted 2 lines

Quoted 2 lines

Are you really suggesting that FP all FP signals should get this treatment?

Quoted 13 lines

feenableexcept(FE_OVERFLOW) kind of means "I can't run safely past an fp overflow exception, please signal me instead".

If the process also blocked SIGFPE, that could be taken to mean "I can't run safely past an fp overflow exception _and_ I can't take SIGFPE either" ... i.e., if an fp overflow happens there is no way to proceed and it's really fatal.

What SIG_IGN ought to mean is rather more debatable, but again, the process could be asking for two opposite things: guarantee a SIGFPE is delivered instead of running past an fp exception, and also guarantee that SIGFPE is _not_ delivered.

Quoted 5 lines

It looks like arm and arm64 are different from most other arches (including x86) here, but I'm not sure what is considered correct, and it looks like the answer is not standardised. There's a possibility that some software goes subtly wrong on arm/arm64 where on other arches it would get terminated with SIGKILL.

Quoted 5 lines

Whether this matters depends on how harmless the fp exception is to the work of the program. I think if an exception is set to trap via feenableexcept() then that's a strong hint the programmer thinks that exception is not harmless. OTOH, trapping is not always available anyway...

Like many of these things, there is no clear answer. It's a set of conflicting requirements, and as you point out, even if you've called feenableexcept(), you are not guaranteed to get a trap.

However, do remember that FP exceptions on ARM hardware are already asynchronous - they get reported by the _next_ FP operation to the one that caused them, which means they could be raised by a library function sometime after it occured (when the library function decides to save the FP registers to the stack before it makes use of them.) It's entirely possible that the library function has blocked FP signals temporarily (not explicitly, just decided to block all signals while it does something sensitive) and will unblock them again afterwards - at which point we get the SIGFPE, and it would be quite right to deliver that signal to the user SIGFPE handler, rather than forcing it onto the program mid-library function.

It's also possible that SIGFPE could be blocked by another signal handler having been invoked, and it triggers the latent generation of the SIGFPE.

I'd be more inclined to agree with you if VFP exceptions were synchronous but they aren't.

Quoted 2 lines

Was there some particular program being broken by the force_sig_info() here?

I don't recall.

commit da41119af78864d27ccbf505949df788d5e8aaf5

Author: Russell King <[email protected]>
Date:   Wed Jun 29 23:02:02 2005 +0100

[PATCH] ARM: Don't force SIGFPE

We were forcing SIGFPE on to a user program for no good reason. Use send_sig_info() instead.

Signed-off-by Russell King <[email protected]>

Quoted 1 line · Russell King - ARM Linux

From your description there still seems to be an association with an

instruction so I don't know if I would really call the signal asynchronous. It sounds like the exception is delayed and not asynchronous.

Quoted 5 lines · Eric W. Biederman

Quoted 1 line · Russell King - ARM Linux

From your description there still seems to be an association with an

instruction so I don't know if I would really call the signal asynchronous. It sounds like the exception is delayed and not asynchronous.

Traps can only be passed from ARM coprocessors by a coprocessor refusing to execute an instruction. That's what happens in this case - the VFP gets offered an instruction to execute. It accepts it, and the CPU continues, leaving the VFP to execute its instruction independently. If this instruction generates an error, then nothing happens at this point.

That error remains pending until the CPU offers the coprocessor the next VFP instruction, which it refuses. That causes an undefined instruction exception, and we trap into the kernel VFP code which reads the VFP status and works out what needs to be done.

What this means is that if you execute a VFP instruction, wait 10 minutes and then execute another VFP instruction, if the first VFP instruction raised an exception, you'll get to hear about it 10 minutes later.

You can use whatever weasel words you want to describe that situation, my choice is "asynchronous", your choice is "delayed". However, it is clearly not "synchronous", and arguing that we should report something synchronously that is not reported to _us_ synchronously (where synchronous means "at the same time") is IMHO daft.

So, let's take an example:

installs SIGFPE handler
..fp instructions.. one of which raises an exception returns to main loop
main loop blocks all signals while it sets stuff up calls ppoll()

In the synchronous SIGFPE delivery case, the SIGFPE handler will be called when the exception is generated in the FP code, and delivered at that time. The fact that the main loop blocks all signals happens later, so the users handler gets called as one expects.

In the VFP case, however, the FP instructions towards the end may not end up causing the exception to be signalled until sometime later, and as I've already explained, that may be the result of a C library function accessing the VFP registers. This could well end up trying to deliver the SIGFPE while signals are blocked, and we get drastically different behaviour if force_sig_info() is used.

In the VFP case, if force_sig_info() is used, the program gets killed at this point. In the non-VFP case, the program's signal handler was called.

Using send_sig_info() results in the already delayed or asynchronous signal being held off until ppoll() drops the blocking, at which point the signal is delivered, the program handles it in its handler, and the program continues to run.

So
1. non-VFP case, program doesn't get killed but gets the opportunity to handle the signal.
2. VFP case with send_sig_info, program doesn't get killed but gets the opportunity to handle the signal.
3. VFP case with force_sig_info, the program gets killed and dumps core.

Which one of these results in a big change of behaviour in your opinion?

Quoted 1 line · Dave Martin

On Thu, Jan 11, 2018 at 06:59:36PM -0600, Eric W. Biederman wrote:

Quoted 3 lines

Quoted 3 lines

Possible ABI fixes include:
- Send the signal without siginfo
- Don't generate a signal

Quoted 8 lines

Quoted 2 lines

- Possibly assign and use an appropriate si_code
- Don't handle cases which can't happen

I think a mixture of these two is the best approach.

In any case, si_code == 0 here doesn't seem to have any explicit meaning. I think we can translate all of the arm64 faults to proper si_codes -- see my sketch below. Probably means a bit more thought though.

Quoted 1 line

Quoted 1 line

arch/arm64/mm/fault.c +0 -0

Quoted 8 lines

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 613

Quoted 8 lines

arch/arm64/mm/fault.c +0 -4

@@ -607,70 +607,70 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 }
 
 static const struct fault_info fault_info[] = {
-	{ do_bad,		SIGBUS,  0,		"ttbr address size fault"	},
-	{ do_bad,		SIGBUS,  0,		"level 1 address size fault"	},
-	{ do_bad,		SIGBUS,  0,		"level 2 address size fault"	},
-	{ do_bad,		SIGBUS,  0,		"level 3 address size fault"	},

Quoted 3 lines · Eric W. Biederman

Quoted 1 line · Dave Martin

On Thu, Jan 11, 2018 at 06:59:36PM -0600, Eric W. Biederman wrote:

[...]

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Possible ABI fixes include:
- Send the signal without siginfo
- Don't generate a signal

[...]

Quoted 8 lines

Quoted 8 lines

Quoted 2 lines

- Possibly assign and use an appropriate si_code
- Don't handle cases which can't happen

I think a mixture of these two is the best approach.

In any case, si_code == 0 here doesn't seem to have any explicit meaning. I think we can translate all of the arm64 faults to proper si_codes -- see my sketch below. Probably means a bit more thought though.

[...]

Quoted 1 line

Quoted 1 line

Quoted 1 line

arch/arm64/mm/fault.c +0 -0

[...]

Quoted 8 lines

Quoted 8 lines

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 613

Quoted 8 lines

arch/arm64/mm/fault.c +0 -4

@@ -607,70 +607,70 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 }
 
 static const struct fault_info fault_info[] = {
-	{ do_bad,		SIGBUS,  0,		"ttbr address size fault"	},
-	{ do_bad,		SIGBUS,  0,		"level 1 address size fault"	},
-	{ do_bad,		SIGBUS,  0,		"level 2 address size fault"	},
-	{ do_bad,		SIGBUS,  0,		"level 3 address size fault"	},

If I convert this kind of thing to SIGKILL there really is nothing sensible to put in si_code, except possibly SI_KERNEL (indicating that the kill did not come from userspace). Even so, it hardly seems worth filling in fields like si_pid and si_uid just to make this "correct".

In any case, if siginfo is never seen by userspace for SIGKILL this is moot.

Obviously, siginfo is never copied to the user stack in that case, but is it also guaranteed not to be visible to userspace by other means? For ptrace I'm hoping not, since SIGKILL should nuke the tracee immediately instead of being reported to the tracer as a signal-delivery-stop -- so the tracer should get WIFSIGNALED() && WTERMSIG() == SIGKILL. A subsequent PTRACE_GETSIGINFO would fail with ESRCH.

Does that match your understanding?

If so, there is some merit in not pretending to pass a reall value for si_code.

Should si_code simply be ignored for the SIGKILL case?

Dave Martin <[email protected]> writes:

Quoted 1 line

Should si_code simply be ignored for the SIGKILL case?

I know what x86 does in a similar case is it uses force_sig instead of force_sig_info. Then the generic code gets to worry about

If the appropriate paths generic paths get to worry about what siginfo to fill in in that case. Which for SI_KERNEL is zero for everything except the si_code and the si_signo.

That seems perfectly reasonable.

Quoted 5 lines · Eric W. Biederman

arch/arm64/mm/fault.c +0 -0

index 9b7f89df49db..abe200587334 100644
@@ -596,7 +596,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 626

Quoted 1 line

arch/arm64/mm/fault.c +1 -0

+	{ do_sea,		SIGBUS,  BUS_FIXME,	"synchronous external abort"	},

This si_code seems to be a fallback for if ACPI is absent or doesn't know what to do with this error.

-> SIGBUS/BUS_OBJERR?

Can probably legitimately happen for userspace for suitable MMIO mappings.

Perhaps it's more serious though in the presence of ACPI. Do we expect that ACPI can diagnose all localisable errors?

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 633

Quoted 4 lines

arch/arm64/mm/fault.c +4 -0

+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 0 (translation table walk)"	},
+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 1 (translation table walk)"	},
+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 2 (translation table walk)"	},
+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 3 (translation table walk)"	},

Pagetable screwup or kernel/system/CPU bug -> SIGKILL, or panic().

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 634

Quoted 1 line

arch/arm64/mm/fault.c +1 -0

+	{ do_sea,		SIGBUS,  BUS_FIXME,	"synchronous parity or ECC error" },	// Reserved when RAS is implemented

Possibly SIGBUS/BUS_MCEERR_AR (though I don't know exactly what userspace is supposed to do with this or whether this implies the existence or certain kernel features for managing the error that may not be present on arm64...)

Otherwise, SIGKILL.

arch/arm64/mm/fault.c static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) · line 641

Quoted 4 lines

arch/arm64/mm/fault.c +4 -0

+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 0 synchronous parity error (translation table walk)"	},	// Reserved when RAS is implemented
+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 1 synchronous parity error (translation table walk)"	},	// Reserved when RAS is implemented
+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 2 synchronous parity error (translation table walk)"	},	// Reserved when RAS is implemented
+	{ do_sea,		SIGBUS,  BUS_FIXME,	"level 3 synchronous parity error (translation table walk)"	},	// Reserved when RAS is implemented

Process page tables corrupt: if the kernel couldn't fix this, the process can't reasonably fix it -> SIGKILL

Since this is a RAS-type error it could be triggered by a cosmic ray rather than requiring a kernel or system bug or other major failure, so we probably shouldn't panic the system if the error is localisable to a particular process.

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Utilizing FPE_FIXME, siginfo_layout will now return SIL_FAULT and the appropriate fields will be reliably copied.

Quoted 5 lines · Eric W. Biederman

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

Quoted 8 lines

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Utilizing FPE_FIXME, siginfo_layout will now return SIL_FAULT and the appropriate fields will be reliably copied.

So what do you suggest when none of the SIGFPE FPE_xxx codes match the condition that "we don't know what happened" ? Raise a SIGKILL instead maybe? We will have dumped the VFP state into the kernel log at this point, things are pretty much fscked.

It's probably an impossible condition unless the hardware has failed, no one has knowingly reported getting such a dump in their kernel log, so it's something that could very likely be changed in some way without anyone noticing.

Quoted 5 lines · Eric W. Biederman

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

Quoted 8 lines

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Utilizing FPE_FIXME, siginfo_layout will now return SIL_FAULT and the appropriate fields will be reliably copied.

So what do you suggest when none of the SIGFPE FPE_xxx codes match the condition that "we don't know what happened" ? Raise a SIGKILL instead maybe? We will have dumped the VFP state into the kernel log at this point, things are pretty much fscked.

It's probably an impossible condition unless the hardware has failed, no one has knowingly reported getting such a dump in their kernel log, so it's something that could very likely be changed in some way without anyone noticing.

Quoted 5 lines · Eric W. Biederman

Setting si_code to 0 results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI.

Quoted 8 lines

Further use of si_code == 0 guaranteed that copy_siginfo_to_user saw a value of __SI_KILL and now sees a value of SIL_KILL with the result that uid and pid fields are copied and which might copying the si_addr field by accident but certainly not by design. Making this a very flakey implementation.

Utilizing FPE_FIXME, siginfo_layout will now return SIL_FAULT and the appropriate fields will be reliably copied.

So what do you suggest when none of the SIGFPE FPE_xxx codes match the condition that "we don't know what happened" ? Raise a SIGKILL instead maybe? We will have dumped the VFP state into the kernel log at this point, things are pretty much fscked.

It's probably an impossible condition unless the hardware has failed, no one has knowingly reported getting such a dump in their kernel log, so it's something that could very likely be changed in some way without anyone noticing.