[PATCH] telnetd:Fix deadlock on cleanup

* [PATCH] telnetd:Fix deadlock on cleanup
@ 2018-05-28  6:27 Seiichi Ishitsuka
  2018-05-28  8:39 ` Alexander Kanavin
  2018-05-29 17:38 ` [OE-core] " Khem Raj
  0 siblings, 2 replies; 9+ messages in thread
From: Seiichi Ishitsuka @ 2018-05-28  6:27 UTC (permalink / raw)
  To: Openembedded-core

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]

Hi all,

I was able to reproduce the old problems shown below with netkit-telnet_0.17.bb.

https://patchwork.openembedded.org/patch/96727/
https://bugs.launchpad.net/ubuntu/+source/netkit-telnet/+bug/507455

The evaluated environment is,
 glibc  :2.2 
 kernel :4.4.32

How to reproduce:

Run the script more than once in the background, and
please adjust CPU usage to 100%.

#!/bin/sh

HOST="host.target"
USER="root"
PASS="password"

while :
  do
  expect -c "
  set timeout 10
  spawn telnet $HOST
  expect login:\  ; send \"$USER\r\"
  expect sword:\  ; send \"$PASS\r\"
  expect \"# \" ; send \"exit\r\"
  " > /dev/null
  done


Best regards,
Seiichi Ishitsuka

[-- Attachment #2: 0001-telnetd-Fix-deadlock-on-cleanup.patch --]
[-- Type: application/octet-stream, Size: 2957 bytes --]

From cdf8829f743adb3ec72c231ee5843dabed91b30f Mon Sep 17 00:00:00 2001
From: Seiichi Ishitsuka <ishitsuka.sc@ncos.nec.co.jp>
Date: Tue, 8 May 2018 09:53:34 +0900
Subject: [PATCH] telnetd:Fix deadlock on cleanup

the patch comes from:
https://launchpadlibrarian.net/37882973/0001-telnetd-Fix-deadlock-on-cleanup.patch

The cleanup function in telnetd is called both directly and on SIGCHLD
signals. This, unfortunately, triggered a deadlock in glibc 2.2 while
running on a 4.4.32 kernel.

What we were seeing is hangs like these:

(gdb) bt
 #0  0x489fee6c in __lll_lock_wait_private (futex=0x48a5e9a4 <__libc_utmp_lock>) at ./lowlevellock.c:33
 #1  0x48a27934 in __utmpname (file=0x48ac1278 "/var/run/utmp") at utmpname.c:42
 #2  0x48ac0ec0 in logout (line=0x27e9d "pts/11") at logout.c:32

How to reproduce:

Run the script more than once in the background.

 #!/bin/sh

 HOST="host.target"
 USER="root"
 PASS="password"

 while :
 do
   expect -c "
   set timeout 10
   spawn telnet $HOST
   expect login:\  ; send \"$USER\r\"
   expect sword:\  ; send \"$PASS\r\"
   expect \"# \" ; send \"exit\r\"
 " > /dev/null
 done

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.6.4


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