[PATCH 1/8] signal/alpha: Document a conflict with SI_USER for SIGTRAP

[PATCH 1/8] signal/alpha: Document a conflict with SI_USER for SIGTRAP

[Eric W. Biederman]

Setting si_code to __SI_FAULT 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.

Given that alpha is on it's last legs I don't know that it is worth
fixing this, but it is worth documenting what is going on so that
no one decides to copy this bad decision.

This was introduced during the 2.5 development cycle so this
mess has had a long time for people to be able to depend upon it.

Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: linux-alpha@vger.kernel.org
History Tree: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
Ref: 0a635c7a84cf ("Fill in siginfo_t.")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 arch/alpha/include/uapi/asm/siginfo.h | 5 +++++
 arch/alpha/kernel/traps.c             | 6 +++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/siginfo.h b/arch/alpha/include/uapi/asm/siginfo.h
index 9822362a8424..687e4972971b 100644
--- a/arch/alpha/include/uapi/asm/siginfo.h
+++ b/arch/alpha/include/uapi/asm/siginfo.h
@@ -6,4 +6,9 @@
 
 #include <asm-generic/siginfo.h>
 
+/*
+ * SIGTRAP si_codes
+ */
+#define TRAP_FIXME	(__SI_FAULT|0)	/* Broken dup of SI_USER */
+
 #endif
diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c
index 65bb102d985b..37c4fc91b215 100644
--- a/arch/alpha/kernel/traps.c
+++ b/arch/alpha/kernel/traps.c
@@ -278,7 +278,7 @@ do_entIF(unsigned long type, struct pt_regs *regs)
 	      case 1: /* bugcheck */
 		info.si_signo = SIGTRAP;
 		info.si_errno = 0;
-		info.si_code = __SI_FAULT;
+		info.si_code = TRAP_FIXME;
 		info.si_addr = (void __user *) regs->pc;
 		info.si_trapno = 0;
 		send_sig_info(SIGTRAP, &info, current);
@@ -318,7 +318,7 @@ do_entIF(unsigned long type, struct pt_regs *regs)
 			break;
 		case GEN_ROPRAND:
 			signo = SIGFPE;
-			code = __SI_FAULT;
+			code = TRAP_FIXME;
 			break;
 
 		case GEN_DECOVF:
@@ -340,7 +340,7 @@ do_entIF(unsigned long type, struct pt_regs *regs)
 		case GEN_SUBRNG7:
 		default:
 			signo = SIGTRAP;
-			code = __SI_FAULT;
+			code = TRAP_FIXME;
 			break;
 		}
 
-- 
2.10.1

Re: [PATCH 1/8] signal/alpha: Document a conflict with SI_USER for SIGTRAP

[Helge Deller]

* Eric W. Biederman <ebiederm@xmission.com>:
> Setting si_code to __SI_FAULT 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.
> 
> Given that alpha is on it's last legs I don't know that it is worth
> fixing this, but it is worth documenting what is going on so that
> no one decides to copy this bad decision.

The ABI was already broken, so IMHO I think it's better to somehow "fix"
it instead. Agreed, alpha and some other architectures are already aged,
but nevertheless most of them build in debian-ports.  Below is a
suggested fix which reuses/misuses other existing trap codes instead.

Helge

Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c
index 65bb102..2ed37dd 100644
--- a/arch/alpha/kernel/traps.c
+++ b/arch/alpha/kernel/traps.c
@@ -278,7 +278,7 @@ do_entIF(unsigned long type, struct pt_regs *regs)
 	      case 1: /* bugcheck */
 		info.si_signo = SIGTRAP;
 		info.si_errno = 0;
-		info.si_code = __SI_FAULT;
+		info.si_code = TRAP_HWBKPT;
 		info.si_addr = (void __user *) regs->pc;
 		info.si_trapno = 0;
 		send_sig_info(SIGTRAP, &info, current);
@@ -318,7 +318,7 @@ do_entIF(unsigned long type, struct pt_regs *regs)
 			break;
 		case GEN_ROPRAND:
 			signo = SIGFPE;
-			code = __SI_FAULT;
+			code = FPE_FLTSUB;
 			break;
 
 		case GEN_DECOVF:
@@ -340,7 +340,7 @@ do_entIF(unsigned long type, struct pt_regs *regs)
 		case GEN_SUBRNG7:
 		default:
 			signo = SIGTRAP;
-			code = __SI_FAULT;
+			code = TRAP_HWBKPT;
 			break;
 		}
 

Re: [PATCH 1/8] signal/alpha: Document a conflict with SI_USER for SIGTRAP

[Eric W. Biederman]

Helge Deller <deller-Mmb7MZpHnFY@public.gmane.org> writes:

> * Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>:
>> Setting si_code to __SI_FAULT 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.
>> 
>> Given that alpha is on it's last legs I don't know that it is worth
>> fixing this, but it is worth documenting what is going on so that
>> no one decides to copy this bad decision.
>
> The ABI was already broken, so IMHO I think it's better to somehow "fix"
> it instead. Agreed, alpha and some other architectures are already aged,
> but nevertheless most of them build in debian-ports.  Below is a
> suggested fix which reuses/misuses other existing trap codes instead.

The conflict with SI_USER is real.  I don't know that we have any actual
applications that are suffering.

I simply don't have the architecture specific references handy to know
in which cases the oddities will happen let alone test them.  Plus at
least for ia64 and alpha those architectures don't appear to be
receiving updates for new syscalls, and no new hardware is being built
so I don't know how much longer they will last.

That is building for alpha gives:
>   CALL    /home/eric/projects/linux/linux-exit-cleanups/scripts/checksyscalls.sh
> <stdin>:1239:2: warning: #warning syscall seccomp not implemented [-Wcpp]
> <stdin>:1248:2: warning: #warning syscall bpf not implemented [-Wcpp]
> <stdin>:1299:2: warning: #warning syscall userfaultfd not implemented [-Wcpp]
> <stdin>:1302:2: warning: #warning syscall membarrier not implemented [-Wcpp]
> <stdin>:1305:2: warning: #warning syscall mlock2 not implemented [-Wcpp]
> <stdin>:1308:2: warning: #warning syscall copy_file_range not implemented [-Wcpp]
> <stdin>:1311:2: warning: #warning syscall preadv2 not implemented [-Wcpp]
> <stdin>:1314:2: warning: #warning syscall pwritev2 not implemented [-Wcpp]
> <stdin>:1317:2: warning: #warning syscall pkey_mprotect not implemented [-Wcpp]
> <stdin>:1320:2: warning: #warning syscall pkey_alloc not implemented [-Wcpp]
> <stdin>:1323:2: warning: #warning syscall pkey_free not implemented [-Wcpp]
> <stdin>:1326:2: warning: #warning syscall statx not implemented [-Wcpp]

But a real fix like you have shown if you can get it approved by the
alpha architecture maintainers I am more than happy to see, and would
very much prefer.

You certainly noticed that the middle case was SIGFPE when I missed
that.

Unfortunately I don't currently have the competence to judge the
effects of your patch.

Eric

> Helge
>
> Signed-off-by: Helge Deller <deller-Mmb7MZpHnFY@public.gmane.org>
>
> diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c
> index 65bb102..2ed37dd 100644
> --- a/arch/alpha/kernel/traps.c
> +++ b/arch/alpha/kernel/traps.c
> @@ -278,7 +278,7 @@ do_entIF(unsigned long type, struct pt_regs *regs)
>  	      case 1: /* bugcheck */
>  		info.si_signo = SIGTRAP;
>  		info.si_errno = 0;
> -		info.si_code = __SI_FAULT;
> +		info.si_code = TRAP_HWBKPT;
>  		info.si_addr = (void __user *) regs->pc;
>  		info.si_trapno = 0;
>  		send_sig_info(SIGTRAP, &info, current);
> @@ -318,7 +318,7 @@ do_entIF(unsigned long type, struct pt_regs *regs)
>  			break;
>  		case GEN_ROPRAND:
>  			signo = SIGFPE;
> -			code = __SI_FAULT;
> +			code = FPE_FLTSUB;
>  			break;
>  
>  		case GEN_DECOVF:
> @@ -340,7 +340,7 @@ do_entIF(unsigned long type, struct pt_regs *regs)
>  		case GEN_SUBRNG7:
>  		default:
>  			signo = SIGTRAP;
> -			code = __SI_FAULT;
> +			code = TRAP_HWBKPT;
>  			break;
>  		}
>  

Re: [PATCH 1/8] signal/alpha: Document a conflict with SI_USER for SIGTRAP

[Michael Cree]

On Fri, Jul 14, 2017 at 05:59:06AM -0500, Eric W. Biederman wrote:
> in which cases the oddities will happen let alone test them.  Plus at
> least for ia64 and alpha those architectures don't appear to be
> receiving updates for new syscalls, and no new hardware is being built
> so I don't know how much longer they will last.
> 
> That is building for alpha gives:
> >   CALL    /home/eric/projects/linux/linux-exit-cleanups/scripts/checksyscalls.sh
> > <stdin>:1239:2: warning: #warning syscall seccomp not implemented [-Wcpp]
> > <stdin>:1248:2: warning: #warning syscall bpf not implemented [-Wcpp]
> > <stdin>:1299:2: warning: #warning syscall userfaultfd not implemented [-Wcpp]
> > <stdin>:1302:2: warning: #warning syscall membarrier not implemented [-Wcpp]
> > <stdin>:1305:2: warning: #warning syscall mlock2 not implemented [-Wcpp]
> > <stdin>:1308:2: warning: #warning syscall copy_file_range not implemented [-Wcpp]
> > <stdin>:1311:2: warning: #warning syscall preadv2 not implemented [-Wcpp]
> > <stdin>:1314:2: warning: #warning syscall pwritev2 not implemented [-Wcpp]
> > <stdin>:1317:2: warning: #warning syscall pkey_mprotect not implemented [-Wcpp]
> > <stdin>:1320:2: warning: #warning syscall pkey_alloc not implemented [-Wcpp]
> > <stdin>:1323:2: warning: #warning syscall pkey_free not implemented [-Wcpp]
> > <stdin>:1326:2: warning: #warning syscall statx not implemented [-Wcpp]

Patches to wire up most of those syscalls on Alpha was posted recently
along with others to fix module loading, etc., but unfortunately they
do not appear to have been applied during the merge window...

Cheers,
Michael.

Re: [PATCH 1/8] signal/alpha: Document a conflict with SI_USER for SIGTRAP

[Eric W. Biederman]

Michael Cree <mcree-q3Ck4f9/EBK9koe0gwxAeg@public.gmane.org> writes:

> On Fri, Jul 14, 2017 at 05:59:06AM -0500, Eric W. Biederman wrote:
>> in which cases the oddities will happen let alone test them.  Plus at
>> least for ia64 and alpha those architectures don't appear to be
>> receiving updates for new syscalls, and no new hardware is being built
>> so I don't know how much longer they will last.
>> 
>> That is building for alpha gives:
>> >   CALL    /home/eric/projects/linux/linux-exit-cleanups/scripts/checksyscalls.sh
>> > <stdin>:1239:2: warning: #warning syscall seccomp not implemented [-Wcpp]
>> > <stdin>:1248:2: warning: #warning syscall bpf not implemented [-Wcpp]
>> > <stdin>:1299:2: warning: #warning syscall userfaultfd not implemented [-Wcpp]
>> > <stdin>:1302:2: warning: #warning syscall membarrier not implemented [-Wcpp]
>> > <stdin>:1305:2: warning: #warning syscall mlock2 not implemented [-Wcpp]
>> > <stdin>:1308:2: warning: #warning syscall copy_file_range not implemented [-Wcpp]
>> > <stdin>:1311:2: warning: #warning syscall preadv2 not implemented [-Wcpp]
>> > <stdin>:1314:2: warning: #warning syscall pwritev2 not implemented [-Wcpp]
>> > <stdin>:1317:2: warning: #warning syscall pkey_mprotect not implemented [-Wcpp]
>> > <stdin>:1320:2: warning: #warning syscall pkey_alloc not implemented [-Wcpp]
>> > <stdin>:1323:2: warning: #warning syscall pkey_free not implemented [-Wcpp]
>> > <stdin>:1326:2: warning: #warning syscall statx not implemented [-Wcpp]
>
> Patches to wire up most of those syscalls on Alpha was posted recently
> along with others to fix module loading, etc., but unfortunately they
> do not appear to have been applied during the merge window...

Odd.

In general wiring up system calls is something that can even happen in
-rc2 or -rc3 as system calls get added in -rc1 and then the
architectures have a chance to send in the small patches wiring the up.
At least that is how I understand the usual rules for being a non-x86
architecture.

Eric