Re: Softfloat userland needing to properly deliver SIGFPE traps
In article <20110125212931.ga20...@mail.duskware.de>, Martin Husemann wrote: >Any objections? >If not, I'm going to commit this sometime later this week. > Looks good to me. christos
Re: Softfloat userland needing to properly deliver SIGFPE traps
Any objections?
If not, I'm going to commit this sometime later this week.
Martin
Index: sys_sig.c
===
RCS file: /cvsroot/src/sys/kern/sys_sig.c,v
retrieving revision 1.30
diff -c -u -r1.30 sys_sig.c
--- sys_sig.c 10 Jan 2011 04:39:18 - 1.30
+++ sys_sig.c 22 Jan 2011 09:08:53 -
@@ -223,20 +223,22 @@
if ((u_int)ksi->ksi_signo >= NSIG)
return EINVAL;
- if (ksi->ksi_pid != l->l_proc->p_pid)
- return EPERM;
-
- 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 (pid != l->l_proc->p_pid) {
+ if (ksi->ksi_pid != l->l_proc->p_pid)
+ return EPERM;
+
+ 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 (pid > 0) {
/* kill single process */
mutex_enter(proc_lock);
Re: Softfloat userland needing to properly deliver SIGFPE traps
On Sat, Jan 22, 2011 at 10:11:48AM +0100, Martin Husemann wrote:
> How about the variant below - it allows a process to deliver arbitrary
> signals to itself, but only SI_USER/SI_QUEUE variants to foreign processes.
Ooops, here is the patch...
Martin
Index: sys_sig.c
===
RCS file: /cvsroot/src/sys/kern/sys_sig.c,v
retrieving revision 1.30
diff -c -u -r1.30 sys_sig.c
--- sys_sig.c 10 Jan 2011 04:39:18 - 1.30
+++ sys_sig.c 22 Jan 2011 09:08:53 -
@@ -223,20 +223,22 @@
if ((u_int)ksi->ksi_signo >= NSIG)
return EINVAL;
- if (ksi->ksi_pid != l->l_proc->p_pid)
- return EPERM;
-
- 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 (pid != l->l_proc->p_pid) {
+ if (ksi->ksi_pid != l->l_proc->p_pid)
+ return EPERM;
+
+ 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 (pid > 0) {
/* kill single process */
mutex_enter(proc_lock);
Re: Softfloat userland needing to properly deliver SIGFPE traps
On Fri, Jan 14, 2011 at 10:06:09AM -0800, Matt Thomas wrote: > I'm thinking the check should go away. How about the variant below - it allows a process to deliver arbitrary signals to itself, but only SI_USER/SI_QUEUE variants to foreign processes. Perhaps the whole part inside the new if () block should be hidden in kauth instead, but I don't feel like touching that mess right now. > One thing I'm concerned about is whether this signal will directed to > the queueing lwp. Seems to me that for a synthetic fault like this, > that would need to be true. Yes, do we have means to do that already? Martin
Re: Softfloat userland needing to properly deliver SIGFPE traps
On Fri, Jan 14, 2011 at 07:14:43PM +0100, Matthias Drochner wrote: > If you have some basic FPU available, you could just cause > matching SIGFPEs using eg doubles. Eg assign > HUGE_VAL*HUGE_VAL to a double, or divide a double by zero. > The sun libm code does this in some cases. That's certainly an option - but we still need to solve this generally (and also making float_raise MD overridable is a bit of a pain due to all the renaming and include magic, but of course it is possible). Martin
Re: Softfloat userland needing to properly deliver SIGFPE traps
mar...@duskware.de said: > sparc64 uses softfloat for 128 bit long double If you have some basic FPU available, you could just cause matching SIGFPEs using eg doubles. Eg assign HUGE_VAL*HUGE_VAL to a double, or divide a double by zero. The sun libm code does this in some cases. best regards Matthias Forschungszentrum Juelich GmbH 52425 Juelich Sitz der Gesellschaft: Juelich Eingetragen im Handelsregister des Amtsgerichts Dueren Nr. HR B 3498 Vorsitzender des Aufsichtsrats: MinDirig Dr. Karl Eugen Huthmacher Geschaeftsfuehrung: Prof. Dr. Achim Bachem (Vorsitzender), Dr. Ulrich Krafft (stellv. Vorsitzender), Prof. Dr.-Ing. Harald Bolt, Prof. Dr. Sebastian M. Schmidt
Re: Softfloat userland needing to properly deliver SIGFPE traps
On Jan 14, 2011, at 2:01 AM, Martin Husemann 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. Looking at the siginfo documentation, i don't see any references to SI_USER/SI_KERNEL. I don't even see definitions for them. We don't seem to use SI_KERNEL. I'm thinking the check should go away. One thing I'm concerned about is whether this signal will directed to the queueing lwp. Seems to me that for a synthetic fault like this, that would need to be true.
Re: Softfloat userland needing to properly deliver SIGFPE traps
In article <20110114100118.ga21...@mail.duskware.de>,
Martin Husemann 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 - 1.4
>+++ softfloat-specialize 14 Jan 2011 09:40:15 -
>@@ -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 - 1.30
>+++ sys_sig.c 14 Jan 2011 09:40:38 -
>@@ -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
Softfloat userland needing to properly deliver SIGFPE traps
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-specialize26 Sep 2004 21:13:27 - 1.4
+++ softfloat-specialize14 Jan 2011 09:40:15 -
@@ -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 - 1.30
+++ sys_sig.c 14 Jan 2011 09:40:38 -
@@ -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).
Martin
