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 | --- |
Patrick Williams | 520786c | 2023-06-25 16:20:36 -0500 | [diff] [blame] | 50 | Upstream-Status: Pending |
| 51 | |
Brad Bishop | 1a4b7ee | 2018-12-16 17:11:34 -0800 | [diff] [blame] | 52 | telnetd/ext.h | 1 + |
| 53 | telnetd/sys_term.c | 17 ++++++++++++++++- |
| 54 | telnetd/telnetd.c | 2 +- |
| 55 | 3 files changed, 18 insertions(+), 2 deletions(-) |
| 56 | |
| 57 | diff --git a/telnetd/ext.h b/telnetd/ext.h |
| 58 | index b98d6ec..08f9d07 100644 |
| 59 | --- a/telnetd/ext.h |
| 60 | +++ b/telnetd/ext.h |
| 61 | @@ -97,6 +97,7 @@ void add_slc(int, int, int); |
| 62 | void check_slc(void); |
| 63 | void change_slc(int, int, int); |
| 64 | void cleanup(int); |
| 65 | +void cleanup_sighandler(int); |
| 66 | void clientstat(int, int, int); |
| 67 | void copy_termbuf(char *, int); |
| 68 | void deferslc(void); |
| 69 | diff --git a/telnetd/sys_term.c b/telnetd/sys_term.c |
| 70 | index 5b4aa84..c4fb0f7 100644 |
| 71 | --- a/telnetd/sys_term.c |
| 72 | +++ b/telnetd/sys_term.c |
| 73 | @@ -719,7 +719,7 @@ static void addarg(struct argv_stuff *avs, const char *val) { |
| 74 | * This is the routine to call when we are all through, to |
| 75 | * clean up anything that needs to be cleaned up. |
| 76 | */ |
| 77 | -void cleanup(int sig) { |
| 78 | +void cleanup_sighandler(int sig) { |
| 79 | char *p; |
| 80 | (void)sig; |
| 81 | |
| 82 | @@ -742,3 +742,18 @@ void cleanup(int sig) { |
| 83 | shutdown(net, 2); |
| 84 | exit(0); |
| 85 | } |
| 86 | + |
| 87 | +void cleanup(int sig) { |
| 88 | + sigset_t mask, oldmask; |
| 89 | + |
| 90 | + /* Set up the mask of signals to temporarily block. */ |
| 91 | + sigemptyset (&mask); |
| 92 | + sigaddset (&mask, SIGCHLD); |
| 93 | + |
| 94 | + /* Block SIGCHLD while running cleanup */ |
| 95 | + sigprocmask (SIG_BLOCK, &mask, &oldmask); |
| 96 | + |
| 97 | + cleanup_sighandler(sig); |
| 98 | + /* Technically not needed since cleanup_sighandler exits */ |
| 99 | + sigprocmask (SIG_UNBLOCK, &mask, NULL); |
| 100 | +} |
| 101 | diff --git a/telnetd/telnetd.c b/telnetd/telnetd.c |
| 102 | index 9ace838..788919c 100644 |
| 103 | --- a/telnetd/telnetd.c |
| 104 | +++ b/telnetd/telnetd.c |
| 105 | @@ -833,7 +833,7 @@ void telnet(int f, int p) |
| 106 | signal(SIGTTOU, SIG_IGN); |
| 107 | #endif |
| 108 | |
| 109 | - signal(SIGCHLD, cleanup); |
| 110 | + signal(SIGCHLD, cleanup_sighandler); |
| 111 | |
| 112 | #ifdef TIOCNOTTY |
| 113 | { |
| 114 | -- |
| 115 | 2.7.4 |
| 116 | |