Debian Bug report logs - #326709
libgpm can bomb the stack hooking to its own signal handlers

Reply or subscribe to this bug.

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Ron <ron@debian.org>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: libgpm can bomb the stack hooking to its own signal handlers

Date: Mon, 05 Sep 2005 17:44:26 +0930

Package: gpm
Version: 1.19.6-21
Severity: important
Tags: patch

Hi,

libgpm tries to hook to an existing signal handler for
SIGWINCH and SIGSTP, but doesn't check if its own handler
is already installed.

ncurses now loads libgpm dynamically if available and
would seem to call Gpm_Open more than once.  This leads
to libgpm hooking to its own handler and a stack bomb
when the signal is next raised.

The following code demonstrates the problem:

#include <stdlib.h>
#include <signal.h>
#include <ncurses.h>

main()
{
    initscr();
    mousemask( ALL_MOUSE_EVENTS, NULL );
    printw("Hello World !!!");
    refresh();
    getch();
    raise( SIGWINCH );
    endwin();
    return EXIT_SUCCESS;
}

Build with:
$ LDFLAGS="-lncurses" make demo.c

It will segfault if run in a console controlled by gpm.

The following patch addresses it.  I may NMU shortly applying
only this change as debian/patches/060_dont_hook_our_own_hook
unless requested to do otherwise.

cheers,
Ron


--- gpm-1.19.6/src/liblow.c.orig	2005-09-05 16:29:34.000000000 +0930
+++ gpm-1.19.6/src/liblow.c	2005-09-05 17:09:21.000000000 +0930
@@ -353,28 +353,40 @@
     {
       /* itz Wed Dec 16 23:22:16 PST 1998 use sigaction, the old
          code caused a signal loop under XEmacs */
-      struct sigaction sa;
+      struct sigaction sa, snow;
       sigemptyset(&sa.sa_mask);
 
+      /* We must check if the signals have already been hooked by
+       * this routine, else they will hook back to themselves and
+       * bomb the stack the first time one is raised. */
+
 #if (defined(SIGWINCH))
       /* And the winch hook .. */
-      sa.sa_handler = gpm_winch_hook;
-      sa.sa_flags = 0;
-      sigaction(SIGWINCH, &sa, &gpm_saved_winch_hook);
+      if (sigaction(SIGWINCH, NULL, &snow) == 0
+          && snow.sa_handler != gpm_winch_hook )
+      {
+          sa.sa_handler = gpm_winch_hook;
+          sa.sa_flags = 0;
+          sigaction(SIGWINCH, &sa, &gpm_saved_winch_hook);
+      }
 #endif
 
 #if (defined(SIGTSTP))
       if (gpm_flag == 1) {
-        /* Install suspend hook */
-        sa.sa_handler = SIG_IGN;
-        sigaction(SIGTSTP, &sa, &gpm_saved_suspend_hook);
-
-        /* if signal was originally ignored, job control is not supported */
-        if (gpm_saved_suspend_hook.sa_handler != SIG_IGN) {
-          sa.sa_flags = SA_NOMASK;
-          sa.sa_handler = gpm_suspend_hook;
-          sigaction(SIGTSTP, &sa, 0);
-        } /*if*/
+          if (sigaction(SIGTSTP, NULL, &snow) == 0
+              && snow.sa_handler != gpm_suspend_hook )
+          {
+                /* Install suspend hook */
+                sa.sa_handler = SIG_IGN;
+                sigaction(SIGTSTP, &sa, &gpm_saved_suspend_hook);
+
+                /* if signal was originally ignored, job control is not supported */
+                if (gpm_saved_suspend_hook.sa_handler != SIG_IGN) {
+                  sa.sa_flags = SA_NOMASK;
+                  sa.sa_handler = gpm_suspend_hook;
+                  sigaction(SIGTSTP, &sa, 0);
+                } /*if*/
+          }
       } /*if*/
 #endif
 



-- System Information:
Debian Release: testing/unstable
  APT prefers unstable
  APT policy: (500, 'unstable')
Architecture: i386 (i686)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.12
Locale: LANG=C, LC_CTYPE=C (charmap=ANSI_X3.4-1968)

Versions of packages gpm depends on:
ii  debconf [debconf-2.0]         1.4.58     Debian configuration management sy
ii  debianutils                   2.14.3     Miscellaneous utilities specific t
ii  libc6                         2.3.5-6    GNU C Library: Shared libraries an
ii  ucf                           2.001      Update Configuration File: preserv

gpm recommends no packages.

-- debconf information excluded

Message #10 received at 326709@bugs.debian.org (full text, mbox, reply):

From: Ron <ron@debian.org>

To: 326709@bugs.debian.org

Subject: libgpm can bomb the stack hooking to its own signal handler

Date: Mon, 5 Sep 2005 23:43:12 +0930

Ok.  With reference to IRC discussion with peterS, here is a patch
that supersedes the one I posted earlier and which covers a couple
of extra things.

 - Don't explicitly initialise globals to their default value.
 - Handle hooking to both sa_sigaction and sa_handler.
 - use the gpm_flag to guard against multiple addition of the
   SIGWINCH handler when Gpm_Open is called more than once.

   Note: I didn't add a new flag for that after all, gpm_flag
   seems to have been designed to ensure Gpm_Close must be called
   the same number of times as Open, which does seem like what we
   want if both lib(s) and app may open it explicitly.
   There may still be other things that need to be wrapped in that
   unrelated to signal handling.

 - refactor the install_suspend_handler logic, getting rid of a
   bunch of redundant logic in Gpm_Open, and addressing a problem
   we didn't discuss yet in gpm_suspend_hook -- where if the SIGSTP
   handler that was hooked to removed itself when called, or hooked
   something else in etc. then we would blindly re hook to its old
   state when control returned to gpm, leaving a new bomb for the
   next time that signal was raised.  Now we more cleanly pop our
   handler off the chain before re-raising the signal, and push
   it back on when things return.

tested locally as debian/patches/060_signal_hook_bomb

Time for me to get some sleep too though, poke me in the morning
if this raises any new questions.  Mostly its just "sliding tile
puzzle" changes from where we left discussion earlier.

cheers,
Ron


--- gpm-1.19.6/src/liblow.c.orig	2005-09-05 22:56:14.000000000 +0930
+++ gpm-1.19.6/src/liblow.c	2005-09-05 22:57:06.000000000 +0930
@@ -5,6 +5,7 @@
  * Copyright 1994,1995   rubini@linux.it (Alessandro Rubini)
  * Copyright (C) 1998    Ian Zimmerman <itz@rahul.net>
  * Copyright 2001        Nico Schottelius (nico@schottelius.org)
+ * Copyright 2005        Ron (ron@debian.org)
  * 
  * xterm management is mostly by jtklehto@stekt.oulu.fi (Janne Kukonlehto)
  *
@@ -68,23 +69,24 @@
 } Gpm_Stst;
 
 /*....................................... Global variables */
-int gpm_flag=0; /* almost unuseful now */
-int gpm_tried=0;
+int gpm_flag; /* open count for Gpm_Open/Gpm_Close */
+int gpm_tried;
 int gpm_fd=-1;
-int gpm_hflag=0;
-Gpm_Stst *gpm_stack=NULL;
+int gpm_hflag;
+Gpm_Stst *gpm_stack;
 struct timeval gpm_timeout={10,0};
-Gpm_Handler *gpm_handler=NULL;
-void *gpm_data=NULL;
-int gpm_zerobased=0;
-int gpm_visiblepointer=0;
+Gpm_Handler *gpm_handler;
+void *gpm_data;
+int gpm_zerobased;
+int gpm_visiblepointer;
 int gpm_mx, gpm_my; /* max x and y (1-based) to fit margins */
 
 unsigned char    _gpm_buf[6*sizeof(short)];
 unsigned short * _gpm_arg = (unsigned short *)_gpm_buf +1;
 
 int gpm_consolefd=-1;  /* used to invoke ioctl() */
-int gpm_morekeys=0;
+int gpm_morekeys;
+
 /*-------------------------------------------------------------------*/
 static inline int putdata(int where,  Gpm_Connect *what)
 {
@@ -112,13 +114,16 @@
 
 static struct sigaction gpm_saved_winch_hook;
 
-static void gpm_winch_hook (int signum)
+static void gpm_winch_hook (int signum, siginfo_t *info, void *ucontext)
 {
   struct winsize win;
 
   if (SIG_IGN != gpm_saved_winch_hook.sa_handler &&
       SIG_DFL != gpm_saved_winch_hook.sa_handler) {
-    gpm_saved_winch_hook.sa_handler(signum);
+    if (gpm_saved_winch_hook.sa_flags & SA_SIGINFO)
+      gpm_saved_winch_hook.sa_sigaction(signum, info, ucontext);
+    else
+      gpm_saved_winch_hook.sa_handler(signum);
   } /*if*/
   if (ioctl(gpm_consolefd, TIOCGWINSZ, &win) == -1) {
     return;
@@ -139,12 +144,13 @@
 
 static struct sigaction gpm_saved_suspend_hook;
 
-static void gpm_suspend_hook (int signum)
+static void install_suspend_handler();
+
+static void gpm_suspend_hook (int signum, siginfo_t *info, void *ucontext)
 {
   Gpm_Connect gpm_connect;
   sigset_t old_sigset;
   sigset_t new_sigset;
-  struct sigaction sa;
   int success;
 
   sigemptyset (&new_sigset);
@@ -167,10 +173,13 @@
   /* in bardo here */
 
   /* Reincarnation. Prepare for another death early. */
-  sigemptyset(&sa.sa_mask);
-  sa.sa_handler = gpm_suspend_hook;
-  sa.sa_flags = SA_NOMASK;
-  sigaction (SIGTSTP, &sa, 0);
+  /* Note that if the saved handler unplugged itself
+   * while we were gone, then this could still all
+   * fall apart horribly should we try this again,
+   * even with the precautions taken here.
+   * Signal chaining really must be cooperative if
+   * there is any hope of it working reliably. */
+  install_suspend_handler();
 
   /* Pop the gpm stack by closing the useless connection */
   /* but do it only when we know we opened one.. */
@@ -178,6 +187,22 @@
     Gpm_Close ();
   } /*if*/
 }
+
+void install_suspend_handler()
+{
+  struct sigaction sa;
+
+  /* if signal was originally ignored, job control is not supported */
+  if (sigaction(SIGTSTP, NULL, &sa) == 0
+      && sa.sa_sigaction != gpm_suspend_hook
+      && sa.sa_handler != SIG_IGN )
+  {
+    sigemptyset(&sa.sa_mask);
+    sa.sa_sigaction = gpm_suspend_hook;
+    sa.sa_flags = SA_NOMASK | SA_SIGINFO;
+    sigaction (SIGTSTP, &sa, &gpm_saved_suspend_hook);
+  }
+}
 #endif /* SIGTSTP */
 
 /*-------------------------------------------------------------------*/
@@ -354,28 +379,28 @@
       /* itz Wed Dec 16 23:22:16 PST 1998 use sigaction, the old
          code caused a signal loop under XEmacs */
       struct sigaction sa;
-      sigemptyset(&sa.sa_mask);
+
+      /* We must check if the signals have already been hooked by
+       * this routine, else they will hook back to themselves and
+       * bomb the stack the first time one is raised. */
 
 #if (defined(SIGWINCH))
       /* And the winch hook .. */
-      sa.sa_handler = gpm_winch_hook;
-      sa.sa_flags = 0;
-      sigaction(SIGWINCH, &sa, &gpm_saved_winch_hook);
+      if (gpm_flag == 1
+          && sigaction(SIGWINCH, NULL, &sa) == 0
+          && sa.sa_sigaction != gpm_winch_hook )
+      {
+          sigemptyset(&sa.sa_mask);
+          sa.sa_flags = SA_SIGINFO;
+          sa.sa_sigaction = gpm_winch_hook;
+          sigaction(SIGWINCH, &sa, &gpm_saved_winch_hook);
+      }
 #endif
 
 #if (defined(SIGTSTP))
-      if (gpm_flag == 1) {
-        /* Install suspend hook */
-        sa.sa_handler = SIG_IGN;
-        sigaction(SIGTSTP, &sa, &gpm_saved_suspend_hook);
-
-        /* if signal was originally ignored, job control is not supported */
-        if (gpm_saved_suspend_hook.sa_handler != SIG_IGN) {
-          sa.sa_flags = SA_NOMASK;
-          sa.sa_handler = gpm_suspend_hook;
-          sigaction(SIGTSTP, &sa, 0);
-        } /*if*/
-      } /*if*/
+      /* Install suspend hook */
+      if (gpm_flag == 1)
+        install_suspend_handler();
 #endif
 
     } /*if*/

Message #15 received at 326709@bugs.debian.org (full text, mbox, reply):

From: Daniel Jacobowitz <drow@false.org>

To: 326709@bugs.debian.org

Subject: "Fix" in ncurses

Date: Thu, 13 Oct 2005 15:44:18 -0400

To the correct bug report this time...

I'll be uploading ncurses 5.5-1 shortly, which includes a change to only
call Gpm_Open once.  I'm not going to close this bug, though, since it would
probably be good to fix in GPM too.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Message #20 received at 326709@bugs.debian.org (full text, mbox, reply):

From: Florian Kulzer <florian.kulzer+debian@icfo.es>

To: Debian Bug Tracking System <326709@bugs.debian.org>

Subject: gpm: I think I have this problem again with libncursesw5 5.6+20080920-1

Date: Fri, 26 Sep 2008 21:37:25 +0200

Package: gpm
Version: 1.20.4-2
Followup-For: Bug #326709

Hi,

I think this problem may have resurfaced with version 5.6+20080920-1 of
libncursesw5; for example, aptitude now segfaults when I use it in
interactive mode on a tty while gpm is active. Here is the debugging
output:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fc0781126f0 (LWP 7476)]
0x00007fc0778155a0 in cwidget::toplevel::timeout_thread::instance () from /usr/lib/libcwidget.so.3
(gdb) bt
#0  0x00007fc0778155a0 in cwidget::toplevel::timeout_thread::instance () from /usr/lib/libcwidget.so.3
#1  0x00007fc077a341e6 in endwin () from /lib/libncursesw.so.5
#2  0x00007fc0775919aa in cwidget::toplevel::suspend_without_signals () at toplevel.cc:1162
#3  0x00007fc077591b82 in cwidget::toplevel::suspend () at toplevel.cc:1181
#4  0x00007fc077592141 in cwidget::toplevel::shutdown () at toplevel.cc:1192
#5  0x00000000004b3a37 in ui_main () at ui.cc:2764
#6  0x000000000041b4f0 in main (argc=20, argv=0x7fff8013f808) at main.cc:759

Ron's demo code crashes like this:

Hello World !!!
Program received signal SIGSEGV, Segmentation fault.
0x00007fce4e2a8330 in ?? ()
(gdb) bt
#0  0x00007fce4e2a8330 in ?? ()
#1  <signal handler called>
#2  0x00007fce4e6e1ef5 in raise () from /lib/libc.so.6
#3  0x0000000000400845 in main () at demo.c:12

Downgrading libncursesw5 to version 5.6+20080830-1 or disabling the GPM
daemon is sufficient to prevent the segfaults.

Thanks for your time.

-- System Information:
Debian Release: lenny/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing'), (500, 'stable'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.26-flo (SMP w/2 CPU cores; PREEMPT)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages gpm depends on:
ii  debconf [debconf-2.0]         1.5.23     Debian configuration management sy
ii  debianutils                   2.30       Miscellaneous utilities specific t
ii  libc6                         2.7-13     GNU C Library: Shared libraries
ii  libgpm2                       1.20.4-2   General Purpose Mouse - shared lib
ii  lsb-base                      3.2-20     Linux Standard Base 3.2 init scrip
ii  ucf                           3.0010     Update Configuration File: preserv

gpm recommends no packages.

gpm suggests no packages.

-- debconf information:
* gpm/responsiveness:
* gpm/repeat_type: none
* gpm/append:
  gpm/restart: false
* gpm/sample_rate:
* gpm/type: exps2
* gpm/device: /dev/input/mice

Message #29 received at 326709@bugs.debian.org (full text, mbox, reply):

From: Florian Kulzer <florian.kulzer+debian@icfo.es>

To: Debian Bug Tracking System <326709@bugs.debian.org>

Subject: problem gone with libncursesw5 5.6+20080925-1

Date: Tue, 30 Sep 2008 17:34:31 +0200

Package: gpm
Version: 1.20.4-2
Followup-For: Bug #326709

I just want to confirm that my problems stopped after upgrading to
libncursesw5 version 5.6+20080925-1. They seem to have been caused by a
separate issue with ncurses (#500103) after all; sorry for the noise.

Send a report that this bug log contains spam.