i3 - improved tiling WM

Run authentication in different threads

Patch status: needinfo

Patch by Philippe Virouleau

Long description:

This replaces the commit a305e622, and fixes #895 as well as the bugs
introduced by the previous commit (reported in #1090). It uses pthread
and lock-free synchronization with gcc builtins atomic operations.

To apply this patch, use:
curl http://cr.i3wm.org/patch/244/raw.patch | git am

b/i3lock.c

16 
@@ -22,7 +22,6 @@
17 
 #include <string.h>
18 
 #include <ev.h>
19 
 #include <sys/mman.h>
20 
-#include <sys/wait.h>
21 
 #include <X11/XKBlib.h>
22 
 #include <X11/extensions/XKBfile.h>
23 
 #include <xkbcommon/xkbcommon.h>
24 
@@ -32,6 +31,7 @@
25 
 #include "i3lock.h"
26 
 #include "xcb.h"
27 
 #include "cursors.h"
28 
+#include "pthread.h"
29 
 #include "unlock_indicator.h"
30 
 #include "xinerama.h"
31
32 
@@ -45,13 +45,14 @@ static pam_handle_t *pam_handle;
33 
 int input_position = 0;
34 
 /* Holds the password you enter (in UTF-8). */
35 
 static char password[512];
36 
+/* Holds the password to be verified (in UTF-8). */
37 
+static char pam_password[512];
38 
 static bool beep = false;
39 
 bool debug_mode = false;
40 
 static bool dpms = false;
41 
 bool unlock_indicator = true;
42 
 static bool dont_fork = false;
43 
 struct ev_loop *main_loop;
44 
-static struct ev_timer *clear_pam_wrong_timeout;
45 
 extern unlock_state_t unlock_state;
46 
 extern pam_state_t pam_state;
47
48 
@@ -63,6 +64,9 @@ cairo_surface_t *img = NULL;
49 
 bool tile = false;
50 
 bool ignore_empty_password = false;
51
52 
+static int waiting_authentication = 0;
53 
+static int max_waiting_authentication = 4;
54 
+
55 
 /* isutf, u8_dec © 2005 Jeff Bezanson, public domain */
56 
 #define isutf(c) (((c) & 0xC0) != 0x80)
57
58 
@@ -157,11 +161,11 @@ out:
59 
  * cold-boot attacks.
60 
  *
61 
  */
62 
-static void clear_password_memory(void) {
63 
+static void clear_password_memory(char *mem) {
64 
     /* A volatile pointer to the password buffer to prevent the compiler from
65 
      * optimizing this out. */
66 
-    volatile char *vpassword = password;
67 
-    for (int c = 0; c < sizeof(password); c++)
68 
+    volatile char *vpassword = mem;
69 
+    for (int c = 0; c < sizeof(mem); c++)
70 
         /* We store a non-random pattern which consists of the (irrelevant)
71 
          * index plus (!) the value of the beep variable. This prevents the
72 
          * compiler from optimizing the calls away, since the value of 'beep'
73 
@@ -169,28 +173,28 @@ static void clear_password_memory(void) {
74 
         vpassword[c] = c + (int)beep;
75 
 }
76
77 
-
78 
 /*
79 
  * Resets pam_state to STATE_PAM_IDLE 2 seconds after an unsuccesful
80 
  * authentication event.
81 
  *
82 
  */
83 
-static void clear_pam_wrong(EV_P_ ev_timer *w, int revents) {
84 
-    DEBUG("clearing pam wrong\n");
85 
-    pam_state = STATE_PAM_IDLE;
86 
-    unlock_state = STATE_STARTED;
87 
-    redraw_screen();
88 
-
89 
-    /* Now free this timeout. */
90 
-    ev_timer_stop(main_loop, clear_pam_wrong_timeout);
91 
-    free(clear_pam_wrong_timeout);
92 
-    clear_pam_wrong_timeout = NULL;
93 
+static void clear_pam_wrong() {
94 
+    /* Clear the pam_wrong sign and go idle if we are in the correct state */
95 
+    if (__sync_bool_compare_and_swap(
96 
+                &pam_state, STATE_PAM_WRONG, STATE_PAM_IDLE)) {
97 
+        unlock_state = STATE_STARTED;
98 
+        redraw_screen();
99 
+    }
100 
 }
101
102 
-static void clear_input(void) {
103 
+static void reset_password(void) {
104 
     input_position = 0;
105 
-    clear_password_memory();
106 
+    clear_password_memory(password);
107 
     password[input_position] = '\0';
108 
+}
109 
+
110 
+static void clear_input(void) {
111 
+    reset_password();
112
113 
     /* Hide the unlock indicator after a bit if the password buffer is
114 
      * empty. */
115 
@@ -200,78 +204,106 @@ static void clear_input(void) {
116 
     unlock_state = STATE_KEY_PRESSED;
117 
 }
118
119 
-static void auth_failed(void) {
120 
-    if (debug_mode)
121 
-        fprintf(stderr, "Authentication failure\n");
122 
+static void *authenticate(void *arg) {
123 
+    /*
124 
+     * Here we can't just do :
125 
+     * while(__sync_bool_compare_and_swap(&pam_state,
126 
+     *      STATE_PAM_VERIFY, STATE_PAM_VERIFY))
127 
+     *      ;
128 
+     *
129 
+     * And then affect STATE_PAM_VERIFY to pam_state :
130 
+     * we need to do both in one single atomic operation, so we use comparison
131 
+     * on all the other states.
132 
+     *
133 
+     */
134 
+    DEBUG("Waiting for critical section\n");
135 
+    __sync_add_and_fetch(&waiting_authentication, 1);
136 
+    while (!__sync_bool_compare_and_swap(&pam_state, STATE_PAM_IDLE, STATE_PAM_VERIFY)
137 
+            && !__sync_bool_compare_and_swap(&pam_state, STATE_PAM_WRONG, STATE_PAM_VERIFY))
138 
+            ;
139 
+    //Beginning of the Critical Section
140 
+    DEBUG("Entering critical section\n");
141 
+    __sync_sub_and_fetch(&waiting_authentication, 1);
142 
+
143 
+    /* Copy our private password to pam_password and clean */
144 
+    char *the_password = (char *)arg;
145 
+    memcpy(pam_password, the_password, sizeof(the_password));
146 
+    clear_password_memory(the_password);
147 
+    free(the_password);
148 
+
149 
+
150
151 
-    pam_state = STATE_PAM_WRONG;
152 
     redraw_screen();
153
154 
-    /* Clear this state after 2 seconds (unless the user enters another
155 
-     * password during that time). */
156 
-    ev_now_update(main_loop);
157 
-    if ((clear_pam_wrong_timeout = calloc(sizeof(struct ev_timer), 1))) {
158 
-        ev_timer_init(clear_pam_wrong_timeout, clear_pam_wrong, 2.0, 0.);
159 
-        ev_timer_start(main_loop, clear_pam_wrong_timeout);
160 
+    if (pam_authenticate(pam_handle, 0) == PAM_SUCCESS) {
161 
+        DEBUG("successfully authenticated\n");
162 
+        clear_password_memory(password);
163 
+        clear_password_memory(pam_password);
164 
+        exit(0);
165 
     }
166
167 
-    /* Cancel the clear_indicator_timeout, it would hide the unlock indicator
168 
-     * too early. */
169 
-    stop_clear_indicator_timeout();
170 
+    if (debug_mode)
171 
+        fprintf(stderr, "Authentication failure\n");
172 
+
173 
+    /* Clear the pam_password memory (password has already been cleared) */
174 
+    clear_password_memory(pam_password);
175
176 
     /* beep on authentication failure, if enabled */
177 
     if (beep) {
178 
         xcb_bell(conn, 100);
179 
         xcb_flush(conn);
180 
     }
181 
-}
182 
-
183 
-static void child_cb(EV_P_ ev_child *child_watcher, int revents) {
184 
-    if (child_watcher->rstatus != 0) {
185 
-        DEBUG("Authentication successfull\n");
186 
-        clear_password_memory();
187
188 
-        exit(0);
189 
-    } else {
190 
-        auth_failed();
191 
+    if (!__sync_bool_compare_and_swap(&pam_state,
192 
+                STATE_PAM_VERIFY, STATE_PAM_WRONG)) {
193 
+        /*
194 
+         * This is an error : once pam_state = STATE_PAM_VERIFY,
195 
+         * only one thread can access it, so the comparison should
196 
+         * always be true. The atomic operation is here to ensure data
197 
+         * coherency with the main thread.
198 
+         * Can't exit now as it would unlock the screen, so just return.
199 
+         */
200 
+        fprintf(stderr, "i3lock : pam_state is not \"verify\" within\
201 
+                critical section\n");
202 
+        return 0;
203 
     }
204 
-    ev_child_stop(main_loop, child_watcher);
205 
-    free(child_watcher);
206 
+    DEBUG("Leaving critical section\n");
207 
+    //End of Critical Section
208 
+    redraw_screen();
209 
+
210 
+
211 
+    /* Cancel the clear_indicator_timeout, it would hide the unlock indicator
212 
+     * too early. */
213 
+    stop_clear_indicator_timeout();
214 
+
215 
+    /* Wait for 2 seconds and maybe clear the pam_wrong state */
216 
+    sleep(2);
217 
+
218 
+    clear_pam_wrong();
219 
+
220 
+    return 0;
221 
 }
222
223 
 static void input_done(void) {
224 
-    if (pam_state == STATE_PAM_VERIFY) {
225 
+    if (waiting_authentication > max_waiting_authentication)
226 
         return;
227 
-    }
228 
-
229 
-    if (clear_pam_wrong_timeout) {
230 
-        ev_timer_stop(main_loop, clear_pam_wrong_timeout);
231 
-        free(clear_pam_wrong_timeout);
232 
-        clear_pam_wrong_timeout = NULL;
233 
-    }
234 
+    char *threadprivate_password = malloc(sizeof(password));
235 
+    if (threadprivate_password) {
236 
+        memcpy(threadprivate_password, password, sizeof(password));
237
238 
-    pam_state = STATE_PAM_VERIFY;
239 
-    redraw_screen();
240 
+        /* The thread */
241 
+        pthread_t auth_thread;
242
243 
-    /* fork to unblock pam_authenticate */
244 
-    pid_t cpid = fork();
245 
-    if (cpid == 0) {
246 
-        exit(pam_authenticate(pam_handle, 0) == PAM_SUCCESS);
247 
-    } else if (cpid > 0) {
248 
-        clear_input();
249 
-        struct ev_child *child_watcher = calloc(sizeof(struct ev_io), 1);
250 
-        ev_child_init(child_watcher, child_cb, cpid, 0);
251 
-        ev_child_set(child_watcher, cpid, 0);
252 
-        ev_child_start(EV_DEFAULT_ child_watcher);
253 
-    } else if (cpid < 0) {
254 
-        DEBUG("Could not fork");
255 
-        if (pam_authenticate(pam_handle, 0) == PAM_SUCCESS) {
256 
-            DEBUG("successfully authenticated\n");
257 
-            clear_password_memory();
258 
-            exit(0);
259 
+        /* Create the authentication thread, if it fails, do it in main thread */
260 
+        if(pthread_create(&auth_thread, NULL, authenticate, threadprivate_password)) {
261 
+            fprintf(stderr, "Error creating thread, going sequential.\n");
262 
+            authenticate(threadprivate_password);
263 
         }
264 
-        auth_failed();
265 
+    } else {
266 
+        authenticate(password);
267 
     }
268 
+
269 
+    reset_password();
270 
 }
271
272 
 /*
273 
@@ -472,7 +504,7 @@ static int conv_callback(int num_msg, const struct pam_message **msg,
274
275 
         /* return code is currently not used but should be set to zero */
276 
         resp[c]->resp_retcode = 0;
277 
-        if ((resp[c]->resp = strdup(password)) == NULL) {
278 
+        if ((resp[c]->resp = strdup(pam_password)) == NULL) {
279 
             perror("strdup");
280 
             return 1;
281 
         }