[PATCH] telnetd: Fix deadlock on cleanup

* [PATCH] telnetd: Fix deadlock on cleanup
@ 2018-06-08  4:42 Seiichi Ishitsuka
  0 siblings, 0 replies; 8+ messages in thread
From: Seiichi Ishitsuka @ 2018-06-08  4:42 UTC (permalink / raw)
  To: openembedded-devel

The cleanup function in telnetd is called both directly and on SIGCHLD signals.
This triggered a deadlock in glibc and was reproduced in glibc 2.27 while
running on a 4.14.30 kernel.

Signed-off-by: Seiichi Ishitsuka <ishitsuka.sc@ncos.nec.co.jp>
---
 ...01-telnet-telnetd-Fix-deadlock-on-cleanup.patch | 114 +++++++++++++++++++++
 .../netkit-telnet/netkit-telnet_0.17.bb            |   1 +
 2 files changed, 115 insertions(+)
 create mode 100644 meta-networking/recipes-netkit/netkit-telnet/files/0001-telnet-telnetd-Fix-deadlock-on-cleanup.patch

diff --git a/meta-networking/recipes-netkit/netkit-telnet/files/0001-telnet-telnetd-Fix-deadlock-on-cleanup.patch b/meta-networking/recipes-netkit/netkit-telnet/files/0001-telnet-telnetd-Fix-deadlock-on-cleanup.patch
new file mode 100644
index 0000000..945785d
--- /dev/null
+++ b/meta-networking/recipes-netkit/netkit-telnet/files/0001-telnet-telnetd-Fix-deadlock-on-cleanup.patch
@@ -0,0 +1,114 @@
+From 06ed6a6bf25a22902846097d6b6c97e070c2c326 Mon Sep 17 00:00:00 2001
+From: Seiichi Ishitsuka <ishitsuka.sc@ncos.nec.co.jp>
+Date: Fri, 1 Jun 2018 14:27:35 +0900
+Subject: [PATCH] telnetd: Fix deadlock on cleanup
+
+The cleanup function in telnetd is called both directly and on SIGCHLD
+signals. This, unfortunately, triggered a deadlock in eglibc 2.9 while
+running on a 2.6.31.11 kernel.
+
+What we were seeing is hangs like these:
+
+  (gdb) bt
+  #0  0xb7702424 in __kernel_vsyscall ()
+  #1  0xb7658e61 in __lll_lock_wait_private () from ./lib/libc.so.6
+  #2  0xb767e7b5 in _L_lock_15 () from ./lib/libc.so.6
+  #3  0xb767e6e0 in utmpname () from ./lib/libc.so.6
+  #4  0xb76bcde7 in logout () from ./lib/libutil.so.1
+  #5  0x0804c827 in cleanup ()
+  #6  <signal handler called>
+  #7  0xb7702424 in __kernel_vsyscall ()
+  #8  0xb7641003 in __fcntl_nocancel () from ./lib/libc.so.6
+  #9  0xb767e0c3 in getutline_r_file () from ./lib/libc.so.6
+  #10 0xb767d675 in getutline_r () from ./lib/libc.so.6
+  #11 0xb76bce42 in logout () from ./lib/libutil.so.1
+  #12 0x0804c827 in cleanup ()
+  #13 0x0804a0b5 in telnet ()
+  #14 0x0804a9c3 in main ()
+
+and what has happened here is that the user closes the telnet session
+via the escape character. This causes telnetd to call cleanup in frame
+the SIGCHLD signal is delivered while telnetd is executing cleanup.
+
+Telnetd then calls the signal handler for SIGCHLD, which is cleanup().
+Ouch. The actual deadlock is in libc. getutline_r in frame #10 gets the
+__libc_utmp_lock lock, and utmpname above does the same thing in frame
+
+The fix registers the SIGCHLD handler as cleanup_sighandler, and makes
+cleanup disable the SIGCHLD signal before calling cleanup_sighandler.
+
+Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
+
+The patch was imported from the Ubuntu netkit-telnet package.
+(https://bugs.launchpad.net/ubuntu/+source/netkit-telnet/+bug/507455)
+
+A previous patch declaring attributes of functions, but it is not used
+in upstream.
+
+Signed-off-by: Seiichi Ishitsuka <ishitsuka.sc@ncos.nec.co.jp>
+---
+ telnetd/ext.h      |  1 +
+ telnetd/sys_term.c | 17 ++++++++++++++++-
+ telnetd/telnetd.c  |  2 +-
+ 3 files changed, 18 insertions(+), 2 deletions(-)
+
+diff --git a/telnetd/ext.h b/telnetd/ext.h
+index b98d6ec..08f9d07 100644
+--- a/telnetd/ext.h
++++ b/telnetd/ext.h
+@@ -97,6 +97,7 @@ void add_slc(int, int, int);
+ void check_slc(void);
+ void change_slc(int, int, int);
+ void cleanup(int);
++void cleanup_sighandler(int);
+ void clientstat(int, int, int);
+ void copy_termbuf(char *, int);
+ void deferslc(void);
+diff --git a/telnetd/sys_term.c b/telnetd/sys_term.c
+index 5b4aa84..c4fb0f7 100644
+--- a/telnetd/sys_term.c
++++ b/telnetd/sys_term.c
+@@ -719,7 +719,7 @@ static void addarg(struct argv_stuff *avs, const char *val) {
+  * This is the routine to call when we are all through, to
+  * clean up anything that needs to be cleaned up.
+  */
+-void cleanup(int sig) {
++void cleanup_sighandler(int sig) {
+     char *p;
+     (void)sig;
+ 
+@@ -742,3 +742,18 @@ void cleanup(int sig) {
+     shutdown(net, 2);
+     exit(0);
+ }
++
++void cleanup(int sig) {
++    sigset_t mask, oldmask;
++
++    /* Set up the mask of signals to temporarily block. */
++    sigemptyset (&mask);
++    sigaddset (&mask, SIGCHLD);
++
++    /* Block SIGCHLD while running cleanup */
++    sigprocmask (SIG_BLOCK, &mask, &oldmask);
++
++    cleanup_sighandler(sig);
++    /* Technically not needed since cleanup_sighandler exits */
++    sigprocmask (SIG_UNBLOCK, &mask, NULL);
++}
+diff --git a/telnetd/telnetd.c b/telnetd/telnetd.c
+index 9ace838..788919c 100644
+--- a/telnetd/telnetd.c
++++ b/telnetd/telnetd.c
+@@ -833,7 +833,7 @@ void telnet(int f, int p)
+     signal(SIGTTOU, SIG_IGN);
+ #endif
+     
+-    signal(SIGCHLD, cleanup);
++    signal(SIGCHLD, cleanup_sighandler);
+     
+ #ifdef TIOCNOTTY
+     {
+-- 
+2.7.4
+
diff --git a/meta-networking/recipes-netkit/netkit-telnet/netkit-telnet_0.17.bb b/meta-networking/recipes-netkit/netkit-telnet/netkit-telnet_0.17.bb
index 28ef36b..c03b8d9 100644
--- a/meta-networking/recipes-netkit/netkit-telnet/netkit-telnet_0.17.bb
+++ b/meta-networking/recipes-netkit/netkit-telnet/netkit-telnet_0.17.bb
@@ -11,6 +11,7 @@ SRC_URI = "ftp://ftp.uk.linux.org/pub/linux/Networking/netkit/${BP}.tar.gz \
            file://telnet-xinetd \
            file://cross-compile.patch \
            file://0001-telnet-telnetd-Fix-print-format-strings.patch \
+           file://0001-telnet-telnetd-Fix-deadlock-on-cleanup.patch \
            "
 
 UPSTREAM_CHECK_URI = "${DEBIAN_MIRROR}/main/n/netkit-telnet/"
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 8+ messages in thread