[Pcsclite-git-commit] [PCSC] 03/06: Fix signal handler by using only allowed functions

Ludovic Rousseau rousseau at moszumanska.debian.org
Sun Apr 24 17:08:31 UTC 2016


This is an automated email from the git hooks/post-receive script.

rousseau pushed a commit to branch master
in repository PCSC.

commit abe436e38aa58cb1140eff0d497ba721474c7703
Author: Ludovic Rousseau <ludovic.rousseau at free.fr>
Date:   Sun Apr 24 18:46:53 2016 +0200

    Fix signal handler by using only allowed functions
    
    The signals are now treated in a special thread created just for that purpose.
    
    Thanks to Andre Florath for the bug report
    https://lists.alioth.debian.org/pipermail/pcsclite-muscle/Week-of-Mon-20160404/000561.html
    
    [Pcsclite-muscle] pcscd jams when using '--auto-exit'
    
    Andre Florath andre at florath.net
    Sat Apr 9 06:06:44 UTC 2016
    
    Hello!
    
    Since some time I have problems with pcscd. I'm using pcscd in
    conjunction with online banking and after a short period of working it
    stops and jams the banking application.
    
    A 'strace' to the pcscd showed that it is still running somewhere
    deep in the USB stack.
    
    The problem is, when manually running the the pcscd, there is no
    problem at all - only when running from systemd.
    Therefore I searched for the differences and found one: the
    '--auto-exit'. Downloaded the source and had a closer look.
    
    What I understand from the source code is, that when '--auto-exit' is
    given, a SIGALRM is generated which (should) terminate the process.
    
    I have noticed that the signal handler 'signal_trap()' uses some
    function calls that are not allowed in signal handlers; like:
    * syslog()
    * gettimeofday()
    * remove()
    
    Using this creates undefined behavior.
    (Please see 'man 7 signal' for a complete list of system calls that
    are not allowed in signal handlers.)
    
    I found a workaround for the issue.
    Changed the service file to:
    
    ===
    [Unit]
    Description=PC/SC Smart Card Daemon
    
    [Service]
    ExecStart=/usr/sbin/pcscd --foreground --debug -a
    ExecReload=/usr/sbin/pcscd --hotplug
    
    [Install]
    Also=pcscd.socket
    ===
    
    and disabling the pcscd.socket gives me a stable system.
    (Yes - pcscd is now started at boot time and runs the whole time
     - which is fine for me.)
    
    If you need more information, please drop me a note.
    
    Kind regards
    
    Andre
---
 src/pcscdaemon.c | 170 ++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 111 insertions(+), 59 deletions(-)

diff --git a/src/pcscdaemon.c b/src/pcscdaemon.c
index 791f7f6..624e759 100644
--- a/src/pcscdaemon.c
+++ b/src/pcscdaemon.c
@@ -81,6 +81,7 @@ char SocketActivated = FALSE;
 static int ExitValue = EXIT_FAILURE;
 int HPForceReaderPolling = 0;
 static int pipefd[] = {-1, -1};
+static int signal_handler_fd[] = {-1, -1};
 char Add_Serial_In_Name = TRUE;
 char Add_Interface_In_Name = TRUE;
 
@@ -89,7 +90,6 @@ char Add_Interface_In_Name = TRUE;
  */
 static void at_exit(void);
 static void clean_temp_files(void);
-static void signal_reload(int sig);
 static void signal_trap(int);
 static void print_version (void);
 static void print_usage (char const * const);
@@ -152,6 +152,10 @@ static void SVCServiceRunLoop(void)
 			/* Nothing to do in case of a syscall interrupted
 			 * It happens when SIGUSR1 (reload) or SIGINT (Ctrl-C) is received
 			 * We just try again */
+
+			/* we wait a bit so that the signal handler thread can do
+			 * its job and set AraKiri if needed */
+			SYS_USleep(1000);
 			break;
 
 		default:
@@ -162,6 +166,93 @@ static void SVCServiceRunLoop(void)
 	}
 }
 
+/**
+ * thread dedicated to handle signals
+ *
+ * a signal handler can not call any function. See signal(7) for a list
+ * of function that are safe to call from a signal handler.
+ * The functions syslog(), gettimeofday() and remove() are NOT safe.
+ */
+static void *signal_thread(void *arg)
+{
+	(void)arg;
+
+	while (TRUE)
+	{
+		int r;
+		int sig;
+
+		r = read(signal_handler_fd[0], &sig, sizeof sig);
+		if (r < 0)
+		{
+			Log2(PCSC_LOG_ERROR, "read failed: %s", strerror(errno));
+			return NULL;
+		}
+
+		Log2(PCSC_LOG_INFO, "Received signal: %d", sig);
+
+		/* signal for hotplug */
+		if (SIGUSR1 == sig)
+		{
+#ifdef USE_USB
+			if (! AraKiri)
+				HPReCheckSerialReaders();
+#endif
+			/* Reenable the signal handler.
+			 * This is needed on Solaris and HPUX. */
+			(void)signal(SIGUSR1, signal_trap);
+
+			continue;
+		}
+
+		/* do not wait if asked to terminate
+		 * avoids waiting after the reader(s) in shutdown for example */
+		if (SIGTERM == sig)
+		{
+			Log1(PCSC_LOG_INFO, "Direct suicide");
+			at_exit();
+		}
+
+		if (SIGALRM == sig)
+		{
+			/* normal exit without error */
+			ExitValue = EXIT_SUCCESS;
+		}
+
+		/* the signal handler is called several times for the same Ctrl-C */
+		if (AraKiri == FALSE)
+		{
+			Log1(PCSC_LOG_INFO, "Preparing for suicide");
+			AraKiri = TRUE;
+
+			/* if still in the init/loading phase the AraKiri will not be
+			 * seen by the main event loop
+			 */
+			if (Init)
+			{
+				Log1(PCSC_LOG_INFO, "Suicide during init");
+				at_exit();
+			}
+		}
+		else
+		{
+			/* if pcscd do not want to die */
+			static int lives = 2;
+
+			lives--;
+			/* no live left. Something is blocking the normal death. */
+			if (0 == lives)
+			{
+				Log1(PCSC_LOG_INFO, "Forced suicide");
+				at_exit();
+			}
+		}
+	}
+
+	return NULL;
+}
+
+
 int main(int argc, char **argv)
 {
 	int rv;
@@ -515,6 +606,20 @@ int main(int argc, char **argv)
 	/* exits on SIGALARM to allow pcscd to suicide if not used */
 	(void)signal(SIGALRM, signal_trap);
 
+	if (pipe(signal_handler_fd) == -1)
+	{
+		Log2(PCSC_LOG_CRITICAL, "pipe() failed: %s", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	pthread_t signal_handler_thread;
+	rv = pthread_create(&signal_handler_thread, NULL, signal_thread, NULL);
+	if (rv)
+	{
+		Log2(PCSC_LOG_CRITICAL, "pthread_create failed: %s", strerror(rv));
+		return EXIT_FAILURE;
+	}
+
 	/*
 	 * If PCSCLITE_IPC_DIR does not exist then create it
 	 */
@@ -610,7 +715,7 @@ int main(int argc, char **argv)
 	/*
 	 * Hotplug rescan
 	 */
-	(void)signal(SIGUSR1, signal_reload);
+	(void)signal(SIGUSR1, signal_trap);
 
 	/*
 	 * Initialize the comm structure
@@ -730,66 +835,13 @@ static void clean_temp_files(void)
 			strerror(errno));
 }
 
-static void signal_reload(/*@unused@*/ int sig)
-{
-	(void)signal(SIGUSR1, signal_reload);
-
-	(void)sig;
-
-	if (AraKiri)
-		return;
-
-#ifdef USE_USB
-	HPReCheckSerialReaders();
-#endif
-} /* signal_reload */
-
 static void signal_trap(int sig)
 {
-	Log2(PCSC_LOG_INFO, "Received signal: %d", sig);
-
-	/* do not wait if asked to terminate
-	 * avoids waiting after the reader(s) in shutdown for example */
-	if (SIGTERM == sig)
-	{
-		Log1(PCSC_LOG_INFO, "Direct suicide");
-		at_exit();
-	}
-
-	if (SIGALRM == sig)
-	{
-		/* normal exit without error */
-		ExitValue = EXIT_SUCCESS;
-	}
-
-	/* the signal handler is called several times for the same Ctrl-C */
-	if (AraKiri == FALSE)
-	{
-		Log1(PCSC_LOG_INFO, "Preparing for suicide");
-		AraKiri = TRUE;
-
-		/* if still in the init/loading phase the AraKiri will not be
-		 * seen by the main event loop
-		 */
-		if (Init)
-		{
-			Log1(PCSC_LOG_INFO, "Suicide during init");
-			at_exit();
-		}
-	}
-	else
-	{
-		/* if pcscd do not want to die */
-		static int lives = 2;
+	int r;
 
-		lives--;
-		/* no live left. Something is blocking the normal death. */
-		if (0 == lives)
-		{
-			Log1(PCSC_LOG_INFO, "Forced suicide");
-			at_exit();
-		}
-	}
+	r = write(signal_handler_fd[1], &sig, sizeof sig);
+	if (r < 0)
+		Log2(PCSC_LOG_ERROR, "write failed: %s", strerror(errno));
 }
 
 static void print_version (void)

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pcsclite/PCSC.git



More information about the Pcsclite-cvs-commit mailing list