|
1 Upstream fix that will be included in the another release of pcsclite. |
|
2 |
|
3 From abe436e38aa58cb1140eff0d497ba721474c7703 Mon Sep 17 00:00:00 2001 |
|
4 From: Ludovic Rousseau <[email protected]> |
|
5 Date: Sun, 24 Apr 2016 18:46:53 +0200 |
|
6 Subject: [PATCH] Fix signal handler by using only allowed functions |
|
7 |
|
8 The signals are now treated in a special thread created just for that purpose. |
|
9 |
|
10 Thanks to Andre Florath for the bug report |
|
11 https://lists.alioth.debian.org/pipermail/pcsclite-muscle/Week-of-Mon-20160404/000561.html |
|
12 |
|
13 [Pcsclite-muscle] pcscd jams when using '--auto-exit' |
|
14 |
|
15 Andre Florath andre at florath.net |
|
16 Sat Apr 9 06:06:44 UTC 2016 |
|
17 |
|
18 Hello! |
|
19 |
|
20 Since some time I have problems with pcscd. I'm using pcscd in |
|
21 conjunction with online banking and after a short period of working it |
|
22 stops and jams the banking application. |
|
23 |
|
24 A 'strace' to the pcscd showed that it is still running somewhere |
|
25 deep in the USB stack. |
|
26 |
|
27 The problem is, when manually running the the pcscd, there is no |
|
28 problem at all - only when running from systemd. |
|
29 Therefore I searched for the differences and found one: the |
|
30 '--auto-exit'. Downloaded the source and had a closer look. |
|
31 |
|
32 What I understand from the source code is, that when '--auto-exit' is |
|
33 given, a SIGALRM is generated which (should) terminate the process. |
|
34 |
|
35 I have noticed that the signal handler 'signal_trap()' uses some |
|
36 function calls that are not allowed in signal handlers; like: |
|
37 * syslog() |
|
38 * gettimeofday() |
|
39 * remove() |
|
40 |
|
41 Using this creates undefined behavior. |
|
42 (Please see 'man 7 signal' for a complete list of system calls that |
|
43 are not allowed in signal handlers.) |
|
44 |
|
45 I found a workaround for the issue. |
|
46 Changed the service file to: |
|
47 |
|
48 === |
|
49 [Unit] |
|
50 Description=PC/SC Smart Card Daemon |
|
51 |
|
52 [Service] |
|
53 ExecStart=/usr/sbin/pcscd --foreground --debug -a |
|
54 ExecReload=/usr/sbin/pcscd --hotplug |
|
55 |
|
56 [Install] |
|
57 Also=pcscd.socket |
|
58 === |
|
59 |
|
60 and disabling the pcscd.socket gives me a stable system. |
|
61 (Yes - pcscd is now started at boot time and runs the whole time |
|
62 - which is fine for me.) |
|
63 |
|
64 If you need more information, please drop me a note. |
|
65 |
|
66 Kind regards |
|
67 |
|
68 Andre |
|
69 --- |
|
70 src/pcscdaemon.c | 170 ++++++++++++++++++++++++++++++++++++------------------- |
|
71 1 file changed, 111 insertions(+), 59 deletions(-) |
|
72 |
|
73 diff --git a/src/pcscdaemon.c b/src/pcscdaemon.c |
|
74 index 791f7f6..624e759 100644 |
|
75 --- a/src/pcscdaemon.c |
|
76 +++ b/src/pcscdaemon.c |
|
77 @@ -81,6 +81,7 @@ char SocketActivated = FALSE; |
|
78 static int ExitValue = EXIT_FAILURE; |
|
79 int HPForceReaderPolling = 0; |
|
80 static int pipefd[] = {-1, -1}; |
|
81 +static int signal_handler_fd[] = {-1, -1}; |
|
82 char Add_Serial_In_Name = TRUE; |
|
83 char Add_Interface_In_Name = TRUE; |
|
84 |
|
85 @@ -89,7 +90,6 @@ char Add_Interface_In_Name = TRUE; |
|
86 */ |
|
87 static void at_exit(void); |
|
88 static void clean_temp_files(void); |
|
89 -static void signal_reload(int sig); |
|
90 static void signal_trap(int); |
|
91 static void print_version (void); |
|
92 static void print_usage (char const * const); |
|
93 @@ -152,6 +152,10 @@ static void SVCServiceRunLoop(void) |
|
94 /* Nothing to do in case of a syscall interrupted |
|
95 * It happens when SIGUSR1 (reload) or SIGINT (Ctrl-C) is received |
|
96 * We just try again */ |
|
97 + |
|
98 + /* we wait a bit so that the signal handler thread can do |
|
99 + * its job and set AraKiri if needed */ |
|
100 + SYS_USleep(1000); |
|
101 break; |
|
102 |
|
103 default: |
|
104 @@ -162,6 +166,93 @@ static void SVCServiceRunLoop(void) |
|
105 } |
|
106 } |
|
107 |
|
108 +/** |
|
109 + * thread dedicated to handle signals |
|
110 + * |
|
111 + * a signal handler can not call any function. See signal(7) for a list |
|
112 + * of function that are safe to call from a signal handler. |
|
113 + * The functions syslog(), gettimeofday() and remove() are NOT safe. |
|
114 + */ |
|
115 +static void *signal_thread(void *arg) |
|
116 +{ |
|
117 + (void)arg; |
|
118 + |
|
119 + while (TRUE) |
|
120 + { |
|
121 + int r; |
|
122 + int sig; |
|
123 + |
|
124 + r = read(signal_handler_fd[0], &sig, sizeof sig); |
|
125 + if (r < 0) |
|
126 + { |
|
127 + Log2(PCSC_LOG_ERROR, "read failed: %s", strerror(errno)); |
|
128 + return NULL; |
|
129 + } |
|
130 + |
|
131 + Log2(PCSC_LOG_INFO, "Received signal: %d", sig); |
|
132 + |
|
133 + /* signal for hotplug */ |
|
134 + if (SIGUSR1 == sig) |
|
135 + { |
|
136 +#ifdef USE_USB |
|
137 + if (! AraKiri) |
|
138 + HPReCheckSerialReaders(); |
|
139 +#endif |
|
140 + /* Reenable the signal handler. |
|
141 + * This is needed on Solaris and HPUX. */ |
|
142 + (void)signal(SIGUSR1, signal_trap); |
|
143 + |
|
144 + continue; |
|
145 + } |
|
146 + |
|
147 + /* do not wait if asked to terminate |
|
148 + * avoids waiting after the reader(s) in shutdown for example */ |
|
149 + if (SIGTERM == sig) |
|
150 + { |
|
151 + Log1(PCSC_LOG_INFO, "Direct suicide"); |
|
152 + at_exit(); |
|
153 + } |
|
154 + |
|
155 + if (SIGALRM == sig) |
|
156 + { |
|
157 + /* normal exit without error */ |
|
158 + ExitValue = EXIT_SUCCESS; |
|
159 + } |
|
160 + |
|
161 + /* the signal handler is called several times for the same Ctrl-C */ |
|
162 + if (AraKiri == FALSE) |
|
163 + { |
|
164 + Log1(PCSC_LOG_INFO, "Preparing for suicide"); |
|
165 + AraKiri = TRUE; |
|
166 + |
|
167 + /* if still in the init/loading phase the AraKiri will not be |
|
168 + * seen by the main event loop |
|
169 + */ |
|
170 + if (Init) |
|
171 + { |
|
172 + Log1(PCSC_LOG_INFO, "Suicide during init"); |
|
173 + at_exit(); |
|
174 + } |
|
175 + } |
|
176 + else |
|
177 + { |
|
178 + /* if pcscd do not want to die */ |
|
179 + static int lives = 2; |
|
180 + |
|
181 + lives--; |
|
182 + /* no live left. Something is blocking the normal death. */ |
|
183 + if (0 == lives) |
|
184 + { |
|
185 + Log1(PCSC_LOG_INFO, "Forced suicide"); |
|
186 + at_exit(); |
|
187 + } |
|
188 + } |
|
189 + } |
|
190 + |
|
191 + return NULL; |
|
192 +} |
|
193 + |
|
194 + |
|
195 int main(int argc, char **argv) |
|
196 { |
|
197 int rv; |
|
198 @@ -515,6 +606,20 @@ int main(int argc, char **argv) |
|
199 /* exits on SIGALARM to allow pcscd to suicide if not used */ |
|
200 (void)signal(SIGALRM, signal_trap); |
|
201 |
|
202 + if (pipe(signal_handler_fd) == -1) |
|
203 + { |
|
204 + Log2(PCSC_LOG_CRITICAL, "pipe() failed: %s", strerror(errno)); |
|
205 + return EXIT_FAILURE; |
|
206 + } |
|
207 + |
|
208 + pthread_t signal_handler_thread; |
|
209 + rv = pthread_create(&signal_handler_thread, NULL, signal_thread, NULL); |
|
210 + if (rv) |
|
211 + { |
|
212 + Log2(PCSC_LOG_CRITICAL, "pthread_create failed: %s", strerror(rv)); |
|
213 + return EXIT_FAILURE; |
|
214 + } |
|
215 + |
|
216 /* |
|
217 * If PCSCLITE_IPC_DIR does not exist then create it |
|
218 */ |
|
219 @@ -610,7 +715,7 @@ int main(int argc, char **argv) |
|
220 /* |
|
221 * Hotplug rescan |
|
222 */ |
|
223 - (void)signal(SIGUSR1, signal_reload); |
|
224 + (void)signal(SIGUSR1, signal_trap); |
|
225 |
|
226 /* |
|
227 * Initialize the comm structure |
|
228 @@ -730,66 +835,13 @@ static void clean_temp_files(void) |
|
229 strerror(errno)); |
|
230 } |
|
231 |
|
232 -static void signal_reload(/*@unused@*/ int sig) |
|
233 -{ |
|
234 - (void)signal(SIGUSR1, signal_reload); |
|
235 - |
|
236 - (void)sig; |
|
237 - |
|
238 - if (AraKiri) |
|
239 - return; |
|
240 - |
|
241 -#ifdef USE_USB |
|
242 - HPReCheckSerialReaders(); |
|
243 -#endif |
|
244 -} /* signal_reload */ |
|
245 - |
|
246 static void signal_trap(int sig) |
|
247 { |
|
248 - Log2(PCSC_LOG_INFO, "Received signal: %d", sig); |
|
249 - |
|
250 - /* do not wait if asked to terminate |
|
251 - * avoids waiting after the reader(s) in shutdown for example */ |
|
252 - if (SIGTERM == sig) |
|
253 - { |
|
254 - Log1(PCSC_LOG_INFO, "Direct suicide"); |
|
255 - at_exit(); |
|
256 - } |
|
257 - |
|
258 - if (SIGALRM == sig) |
|
259 - { |
|
260 - /* normal exit without error */ |
|
261 - ExitValue = EXIT_SUCCESS; |
|
262 - } |
|
263 - |
|
264 - /* the signal handler is called several times for the same Ctrl-C */ |
|
265 - if (AraKiri == FALSE) |
|
266 - { |
|
267 - Log1(PCSC_LOG_INFO, "Preparing for suicide"); |
|
268 - AraKiri = TRUE; |
|
269 - |
|
270 - /* if still in the init/loading phase the AraKiri will not be |
|
271 - * seen by the main event loop |
|
272 - */ |
|
273 - if (Init) |
|
274 - { |
|
275 - Log1(PCSC_LOG_INFO, "Suicide during init"); |
|
276 - at_exit(); |
|
277 - } |
|
278 - } |
|
279 - else |
|
280 - { |
|
281 - /* if pcscd do not want to die */ |
|
282 - static int lives = 2; |
|
283 + int r; |
|
284 |
|
285 - lives--; |
|
286 - /* no live left. Something is blocking the normal death. */ |
|
287 - if (0 == lives) |
|
288 - { |
|
289 - Log1(PCSC_LOG_INFO, "Forced suicide"); |
|
290 - at_exit(); |
|
291 - } |
|
292 - } |
|
293 + r = write(signal_handler_fd[1], &sig, sizeof sig); |
|
294 + if (r < 0) |
|
295 + Log2(PCSC_LOG_ERROR, "write failed: %s", strerror(errno)); |
|
296 } |
|
297 |
|
298 static void print_version (void) |
|
299 -- |
|
300 1.9.1 |
|
301 |
|
302 |