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