Re: Softfloat userland needing to properly deliver SIGFPE traps

[christos%astron.com@localhost (Christos Zoulas)]

In article <20110114100118.GA21401%mail.duskware.de@localhost>,
Martin Husemann  <martin%duskware.de@localhost> wrote:
>After Christos recently added sigqueue and friends to -current, I tried to
>use them to fix a long standing softfloat userland problem. One variant of
>the problem shows up in sparc64 atf test runs (sparc64 uses softfloat
>for 128 bit long double): the userland software, and our atf tests, expect
>to get proper details about the SIGFPE it caught, but softloat only did a
>raise(SIGFPE), so no siginfo is available.
>
>The userland change looked straight forward:
>
>Index: softfloat-specialize
>===================================================================
>RCS file: /cvsroot/src/lib/libc/softfloat/softfloat-specialize,v
>retrieving revision 1.4
>diff -c -u -p -r1.4 softfloat-specialize
>--- softfloat-specialize       26 Sep 2004 21:13:27 -0000      1.4
>+++ softfloat-specialize       14 Jan 2011 09:40:15 -0000
>@@ -56,11 +59,26 @@ should be simply `float_exception_flags 
> fp_except float_exception_mask = 0;
> void float_raise( fp_except flags )
> {
>+    siginfo_t info;
> 
>     float_exception_flags |= flags;
> 
>     if ( flags & float_exception_mask ) {
>-      raise( SIGFPE );
>+      memset(&info, 0, sizeof info);
>+      info.si_signo = SIGFPE;
>+      info.si_pid = getpid();
>+      info.si_uid = geteuid();
>+      if (flags & float_flag_underflow)
>+          info.si_code = FPE_FLTUND;
>+      else if (flags & float_flag_overflow)
>+          info.si_code = FPE_FLTOVF;
>+      else if (flags & float_flag_divbyzero)
>+          info.si_code = FPE_FLTDIV;
>+      else if (flags & float_flag_invalid)
>+          info.si_code = FPE_FLTINV;
>+      else if (flags & float_flag_inexact)
>+          info.si_code = FPE_FLTRES;
>+      sigqueueinfo(getpid(), &info);
>     }
> }
>
>
>But: looking at the kernel code, this is not allowed. Only SI_USER and
>SI_QUEUE request are passed through. For testing, I disabled this
>security check like this:
> 
>Index: sys_sig.c
>===================================================================
>RCS file: /cvsroot/src/sys/kern/sys_sig.c,v
>retrieving revision 1.30
>diff -c -u -p -r1.30 sys_sig.c
>--- sys_sig.c  10 Jan 2011 04:39:18 -0000      1.30
>+++ sys_sig.c  14 Jan 2011 09:40:38 -0000
>@@ -229,14 +229,16 @@ kill1(struct lwp *l, pid_t pid, ksiginfo
>       if (ksi->ksi_uid != kauth_cred_geteuid(l->l_cred))
>               return EPERM;
> 
>-      switch (ksi->ksi_code) {
>-      case SI_USER:
>-      case SI_QUEUE:
>-              break;
>-      default:
>-              return EPERM;
>+      if (ksi->ksi_signo != SIGFPE) {
>+              switch (ksi->ksi_code) {
>+              case SI_USER:
>+              case SI_QUEUE:
>+                      break;
>+              default:
>+                      return EPERM;
>+              }
>       }
>-              
>+
>       if (pid > 0) {
>               /* kill single process */
>               mutex_enter(proc_lock);
>
>
>This change makes it work for me, but of course it is a horrible hack, not
>acceptable for commit. We need to have the possibility to SI_USER/
>SI_QUEUE a SIGFPE, but given the mostly union content of ksiginfo, I
>don't see an obvious way how to do it properly.
>
>I'd like to suggest a special SI_SELFSIGFPE ksi_code, which overrides
>the pid/uid tests, only allows sending to the same process, and encodes
>the final (kernel internal) ksi_code as ksi_signo, while passing on the
>rest of ksiginfo untouched (so userland could, if possible, fill in
>struct fault).
>
>Userland call would look like this:
>
>+      memset(&info, 0, sizeof info);
>+      info.si_code = SI_SELFSIGFPE;
>+      if (flags & float_flag_underflow)
>+          info.si_signo = FPE_FLTUND;
>+      else if (flags & float_flag_overflow)
>+          info.si_signo = FPE_FLTOVF;
>+      else if (flags & float_flag_divbyzero)
>+          info.si_signo = FPE_FLTDIV;
>+      else if (flags & float_flag_invalid)
>+          info.si_signo = FPE_FLTINV;
>+      else if (flags & float_flag_inexact)
>+          info.si_signo = FPE_FLTRES;
>+      sigqueueinfo(getpid(), &info);
>
>and kernel would move si_signo to ksi_code, set signo to SIGFPE, and pass
>the remaining siginfo on untouched.
>
>Still a hack, but I don't have better ideas (besides creating another special
>purpose syscall for this).

I like this. Go for it. And it does not seem to have any security implications
since you only allow it to be sent to self. This needs to be documented of
course in the siginfo man page and SI_SELFSIGFPE should be
#ifdef _NETBSD_SOURCE.

christos