| Brad Bishop | 1a4b7ee | 2018-12-16 17:11:34 -0800 | [diff] [blame] | 1 | From 06ed6a6bf25a22902846097d6b6c97e070c2c326 Mon Sep 17 00:00:00 2001 | 
 | 2 | From: Seiichi Ishitsuka <ishitsuka.sc@ncos.nec.co.jp> | 
 | 3 | Date: Fri, 1 Jun 2018 14:27:35 +0900 | 
 | 4 | Subject: [PATCH] telnetd: Fix deadlock on cleanup | 
 | 5 |  | 
 | 6 | The cleanup function in telnetd is called both directly and on SIGCHLD | 
 | 7 | signals. This, unfortunately, triggered a deadlock in eglibc 2.9 while | 
 | 8 | running on a 2.6.31.11 kernel. | 
 | 9 |  | 
 | 10 | What we were seeing is hangs like these: | 
 | 11 |  | 
 | 12 |   (gdb) bt | 
 | 13 |   #0  0xb7702424 in __kernel_vsyscall () | 
 | 14 |   #1  0xb7658e61 in __lll_lock_wait_private () from ./lib/libc.so.6 | 
 | 15 |   #2  0xb767e7b5 in _L_lock_15 () from ./lib/libc.so.6 | 
 | 16 |   #3  0xb767e6e0 in utmpname () from ./lib/libc.so.6 | 
 | 17 |   #4  0xb76bcde7 in logout () from ./lib/libutil.so.1 | 
 | 18 |   #5  0x0804c827 in cleanup () | 
 | 19 |   #6  <signal handler called> | 
 | 20 |   #7  0xb7702424 in __kernel_vsyscall () | 
 | 21 |   #8  0xb7641003 in __fcntl_nocancel () from ./lib/libc.so.6 | 
 | 22 |   #9  0xb767e0c3 in getutline_r_file () from ./lib/libc.so.6 | 
 | 23 |   #10 0xb767d675 in getutline_r () from ./lib/libc.so.6 | 
 | 24 |   #11 0xb76bce42 in logout () from ./lib/libutil.so.1 | 
 | 25 |   #12 0x0804c827 in cleanup () | 
 | 26 |   #13 0x0804a0b5 in telnet () | 
 | 27 |   #14 0x0804a9c3 in main () | 
 | 28 |  | 
 | 29 | and what has happened here is that the user closes the telnet session | 
 | 30 | via the escape character. This causes telnetd to call cleanup in frame | 
 | 31 | the SIGCHLD signal is delivered while telnetd is executing cleanup. | 
 | 32 |  | 
 | 33 | Telnetd then calls the signal handler for SIGCHLD, which is cleanup(). | 
 | 34 | Ouch. The actual deadlock is in libc. getutline_r in frame #10 gets the | 
 | 35 | __libc_utmp_lock lock, and utmpname above does the same thing in frame | 
 | 36 |  | 
 | 37 | The fix registers the SIGCHLD handler as cleanup_sighandler, and makes | 
 | 38 | cleanup disable the SIGCHLD signal before calling cleanup_sighandler. | 
 | 39 |  | 
 | 40 | Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net> | 
 | 41 |  | 
 | 42 | The patch was imported from the Ubuntu netkit-telnet package. | 
 | 43 | (https://bugs.launchpad.net/ubuntu/+source/netkit-telnet/+bug/507455) | 
 | 44 |  | 
 | 45 | A previous patch declaring attributes of functions, but it is not used | 
 | 46 | in upstream. | 
 | 47 |  | 
 | 48 | Signed-off-by: Seiichi Ishitsuka <ishitsuka.sc@ncos.nec.co.jp> | 
 | 49 | --- | 
 | 50 |  telnetd/ext.h      |  1 + | 
 | 51 |  telnetd/sys_term.c | 17 ++++++++++++++++- | 
 | 52 |  telnetd/telnetd.c  |  2 +- | 
 | 53 |  3 files changed, 18 insertions(+), 2 deletions(-) | 
 | 54 |  | 
 | 55 | diff --git a/telnetd/ext.h b/telnetd/ext.h | 
 | 56 | index b98d6ec..08f9d07 100644 | 
 | 57 | --- a/telnetd/ext.h | 
 | 58 | +++ b/telnetd/ext.h | 
 | 59 | @@ -97,6 +97,7 @@ void add_slc(int, int, int); | 
 | 60 |  void check_slc(void); | 
 | 61 |  void change_slc(int, int, int); | 
 | 62 |  void cleanup(int); | 
 | 63 | +void cleanup_sighandler(int); | 
 | 64 |  void clientstat(int, int, int); | 
 | 65 |  void copy_termbuf(char *, int); | 
 | 66 |  void deferslc(void); | 
 | 67 | diff --git a/telnetd/sys_term.c b/telnetd/sys_term.c | 
 | 68 | index 5b4aa84..c4fb0f7 100644 | 
 | 69 | --- a/telnetd/sys_term.c | 
 | 70 | +++ b/telnetd/sys_term.c | 
 | 71 | @@ -719,7 +719,7 @@ static void addarg(struct argv_stuff *avs, const char *val) { | 
 | 72 |   * This is the routine to call when we are all through, to | 
 | 73 |   * clean up anything that needs to be cleaned up. | 
 | 74 |   */ | 
 | 75 | -void cleanup(int sig) { | 
 | 76 | +void cleanup_sighandler(int sig) { | 
 | 77 |      char *p; | 
 | 78 |      (void)sig; | 
 | 79 |   | 
 | 80 | @@ -742,3 +742,18 @@ void cleanup(int sig) { | 
 | 81 |      shutdown(net, 2); | 
 | 82 |      exit(0); | 
 | 83 |  } | 
 | 84 | + | 
 | 85 | +void cleanup(int sig) { | 
 | 86 | +    sigset_t mask, oldmask; | 
 | 87 | + | 
 | 88 | +    /* Set up the mask of signals to temporarily block. */ | 
 | 89 | +    sigemptyset (&mask); | 
 | 90 | +    sigaddset (&mask, SIGCHLD); | 
 | 91 | + | 
 | 92 | +    /* Block SIGCHLD while running cleanup */ | 
 | 93 | +    sigprocmask (SIG_BLOCK, &mask, &oldmask); | 
 | 94 | + | 
 | 95 | +    cleanup_sighandler(sig); | 
 | 96 | +    /* Technically not needed since cleanup_sighandler exits */ | 
 | 97 | +    sigprocmask (SIG_UNBLOCK, &mask, NULL); | 
 | 98 | +} | 
 | 99 | diff --git a/telnetd/telnetd.c b/telnetd/telnetd.c | 
 | 100 | index 9ace838..788919c 100644 | 
 | 101 | --- a/telnetd/telnetd.c | 
 | 102 | +++ b/telnetd/telnetd.c | 
 | 103 | @@ -833,7 +833,7 @@ void telnet(int f, int p) | 
 | 104 |      signal(SIGTTOU, SIG_IGN); | 
 | 105 |  #endif | 
 | 106 |       | 
 | 107 | -    signal(SIGCHLD, cleanup); | 
 | 108 | +    signal(SIGCHLD, cleanup_sighandler); | 
 | 109 |       | 
 | 110 |  #ifdef TIOCNOTTY | 
 | 111 |      { | 
 | 112 | --  | 
 | 113 | 2.7.4 | 
 | 114 |  |