(VG_(do__NR_sigaction)): Don't allow setting of

handlers for SIGKILL or SIGSTOP.  This fixes
valgrind: vg_signals.c:723 (vgPlain_do__NR_sigaction):
Assertion `our_old_handler == ((void *)0)' failed.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@40

diff --git a/ChangeLog b/ChangeLog
index d51cc8d694e3218bab26a53c9fb510a5115e2c8e..e3265b3e60e1cd9117aa0159f88231b75a9ecde0 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2002-03-29  Julian Seward  <sewardj@localhost.localdomain>
+
+       * vg_signals.c (VG_(do__NR_sigaction)): Don't allow setting of
+       handlers for SIGKILL or SIGSTOP.  This fixes
+       valgrind: vg_signals.c:723 (vgPlain_do__NR_sigaction): 
+       Assertion `our_old_handler == ((void *)0)' failed.
+       
 2002-03-29  Alexandre Duret-Lutz  <duret_g@epita.fr>
 
        * vg_syscall_mem.c: wrapper for ioctl TIOCGPGRP.

diff --git a/coregrind/vg_kerneliface.h b/coregrind/vg_kerneliface.h
index 480188c62250e791894fa2497c46257134edbca8..15ce80d3523501a76d218e185d7361b0bbc26ae1 100644 (file)
--- a/coregrind/vg_kerneliface.h
+++ b/coregrind/vg_kerneliface.h
@@ -132,6 +132,11 @@ typedef
 #define VKI_MAP_PRIVATE    0x02            /* Changes are private.  */
 
 
+/* Copied from /usr/src/linux-2.4.9-13/include/asm/errno.h */
+
+#define VKI_EINVAL          22      /* Invalid argument */
+
+
 /* Gawd ... hack ... */
 
 typedef struct vki__user_cap_header_struct {

diff --git a/coregrind/vg_signals.c b/coregrind/vg_signals.c
index ed7ef67cb7b913674ab0d40f6830dd402b33cdb8..2372fc47383afd39b0f8b10874ce2583785c2cda 100644 (file)
--- a/coregrind/vg_signals.c
+++ b/coregrind/vg_signals.c
@@ -688,7 +688,17 @@ void VG_(do__NR_sigaction) ( void )
          (UInt)(new_action ? new_action->ksa_flags : 0) );
    /* VG_(ppSigProcMask)(); */
 
-   if (param1 < 1 || param1 >= VKI_KNSIG) goto bad;
+   /* Rule out various error conditions.  The aim is to ensure that if
+      the call is passed to the kernel it will definitely succeed. */
+
+   /* Reject out-of-range signal numbers. */
+   if (param1 < 1 || param1 >= VKI_KNSIG) goto bad_signo;
+
+   /* Reject attempts to set a handler (or set ignore) for SIGKILL. */
+   if ( (param1 == VKI_SIGKILL || param1 == VKI_SIGSTOP)
+       && new_action
+       && new_action->ksa_handler != VKI_SIG_DFL)
+      goto bad_sigkill;
 
    our_old_handler = VG_(sighandler)[param1];
    /* VG_(printf)("old handler = 0x%x\n", our_old_handler); */
@@ -714,6 +724,7 @@ void VG_(do__NR_sigaction) ( void )
 
    KERNEL_DO_SYSCALL(res);
    /* VG_(printf)("RES = %d\n", res); */
+
    /* If the client asks for the old handler, maintain our fiction
       by stuffing in the handler it thought it asked for ... */
    if (old_action) {
@@ -742,11 +753,18 @@ void VG_(do__NR_sigaction) ( void )
    VG_(baseBlock)[VGOFF_(m_eax)] = (UInt)0;
    return;
 
-  bad:
+  bad_signo:
    VG_(message)(Vg_UserMsg,
                 "Warning: bad signal number %d in __NR_sigaction.", 
                 param1);
-   VG_(baseBlock)[VGOFF_(m_eax)] = (UInt)(-1);
+   VG_(baseBlock)[VGOFF_(m_eax)] = (UInt)(-VKI_EINVAL);
+   return;
+
+  bad_sigkill:
+   VG_(message)(Vg_UserMsg,
+                "Warning: attempt to set SIGKILL handler in __NR_sigaction.", 
+                param1);
+   VG_(baseBlock)[VGOFF_(m_eax)] = (UInt)(-VKI_EINVAL);
    return;
 }
 

diff --git a/tests/sigkill.c b/tests/sigkill.c
new file mode 100644 (file)
index 0000000..c4c7b77
--- /dev/null
+++ b/tests/sigkill.c
@@ -0,0 +1,35 @@
+
+#include <errno.h>
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+
+static void
+abend (int sig)
+{
+  printf ("Abended on signal %d\n", sig);
+  exit (2);
+}
+
+int
+main (void)
+{
+  struct sigaction  sa;
+
+  int i;
+  for (i = 1; i <= 64; i++) {
+     sa.sa_flags   = 0;
+     sigemptyset( &sa.sa_mask );
+     sa.sa_handler = abend;
+     errno = 0;
+     fprintf(stderr, "setting signal %d: ", i);
+     sigaction (i /*SIGKILL*/, &sa, NULL);
+     perror ("");
+     errno = 0;
+     fprintf(stderr, "getting signal %d: ", i);
+     sigaction (i /*SIGKILL*/, NULL, &sa);
+     perror ("");
+     fprintf(stderr, "\n");
+  }
+  return 0;
+}

diff --git a/tests/signal1.c b/tests/signal1.c
index 68a5cade6a2943e2a5a3e857affb38fc6a5fa64f..a35975c1cfc65abb84c19dc86224aca5b5fcf528 100644 (file)
--- a/tests/signal1.c
+++ b/tests/signal1.c
@@ -11,7 +11,7 @@ void sig_hdlr ( int signo )
    printf ( "signal returns\n" );
 }
 
-void main ( void )
+int main ( void )
 {
    spin = 1;
    printf ( "installing sig handler\n" );
@@ -19,4 +19,5 @@ void main ( void )
    printf ( "entering busy wait\n" );
    while (spin) { };
    printf ( "exited\n" );
+   return 0;
 }

diff --git a/tests/signal2.c b/tests/signal2.c
index 6892d334707a1932bbcc0a5a000e8057225ff583..5797537698d6104d385902744ece3805e07c5d71 100644 (file)
--- a/tests/signal2.c
+++ b/tests/signal2.c
@@ -8,11 +8,12 @@ void sig_hdlr ( int signo )
    exit(1);
 }
 
-void main ( void )
+int main ( void )
 {
    printf ( "installing sig handler\n" );
    signal(SIGSEGV, sig_hdlr);
    printf ( "doing bad thing\n" );
    * (int*) 0 = 0;
    printf ( "exited normally ?!\n" );
+   return 0;
 }

diff --git a/tests/signal3.c b/tests/signal3.c
index e2b4d17b347ec27b474e32edf2c1ccf77fe0eae4..6c1cc68c4f2d5f6955027b46e4038c5a584bc00d 100644 (file)
--- a/tests/signal3.c
+++ b/tests/signal3.c
@@ -26,8 +26,9 @@ void hdp_init_profiling ( void )
    assert(ret == 0);
 }
 
-void main ( void )
+int main ( void )
 {
    hdp_init_profiling();
    while (1) {}
+   return 0;
 }

diff --git a/vg_kerneliface.h b/vg_kerneliface.h
index 480188c62250e791894fa2497c46257134edbca8..15ce80d3523501a76d218e185d7361b0bbc26ae1 100644 (file)
--- a/vg_kerneliface.h
+++ b/vg_kerneliface.h
@@ -132,6 +132,11 @@ typedef
 #define VKI_MAP_PRIVATE    0x02            /* Changes are private.  */
 
 
+/* Copied from /usr/src/linux-2.4.9-13/include/asm/errno.h */
+
+#define VKI_EINVAL          22      /* Invalid argument */
+
+
 /* Gawd ... hack ... */
 
 typedef struct vki__user_cap_header_struct {

diff --git a/vg_signals.c b/vg_signals.c
index ed7ef67cb7b913674ab0d40f6830dd402b33cdb8..2372fc47383afd39b0f8b10874ce2583785c2cda 100644 (file)
--- a/vg_signals.c
+++ b/vg_signals.c
@@ -688,7 +688,17 @@ void VG_(do__NR_sigaction) ( void )
          (UInt)(new_action ? new_action->ksa_flags : 0) );
    /* VG_(ppSigProcMask)(); */
 
-   if (param1 < 1 || param1 >= VKI_KNSIG) goto bad;
+   /* Rule out various error conditions.  The aim is to ensure that if
+      the call is passed to the kernel it will definitely succeed. */
+
+   /* Reject out-of-range signal numbers. */
+   if (param1 < 1 || param1 >= VKI_KNSIG) goto bad_signo;
+
+   /* Reject attempts to set a handler (or set ignore) for SIGKILL. */
+   if ( (param1 == VKI_SIGKILL || param1 == VKI_SIGSTOP)
+       && new_action
+       && new_action->ksa_handler != VKI_SIG_DFL)
+      goto bad_sigkill;
 
    our_old_handler = VG_(sighandler)[param1];
    /* VG_(printf)("old handler = 0x%x\n", our_old_handler); */
@@ -714,6 +724,7 @@ void VG_(do__NR_sigaction) ( void )
 
    KERNEL_DO_SYSCALL(res);
    /* VG_(printf)("RES = %d\n", res); */
+
    /* If the client asks for the old handler, maintain our fiction
       by stuffing in the handler it thought it asked for ... */
    if (old_action) {
@@ -742,11 +753,18 @@ void VG_(do__NR_sigaction) ( void )
    VG_(baseBlock)[VGOFF_(m_eax)] = (UInt)0;
    return;
 
-  bad:
+  bad_signo:
    VG_(message)(Vg_UserMsg,
                 "Warning: bad signal number %d in __NR_sigaction.", 
                 param1);
-   VG_(baseBlock)[VGOFF_(m_eax)] = (UInt)(-1);
+   VG_(baseBlock)[VGOFF_(m_eax)] = (UInt)(-VKI_EINVAL);
+   return;
+
+  bad_sigkill:
+   VG_(message)(Vg_UserMsg,
+                "Warning: attempt to set SIGKILL handler in __NR_sigaction.", 
+                param1);
+   VG_(baseBlock)[VGOFF_(m_eax)] = (UInt)(-VKI_EINVAL);
    return;
 }