ipcpd: Mitigate race in wait_req_arr and wait_resp

The wait_flow_req_arr and wait_flow_resp functions are there to ensure
the responses of the IRMd to flow allocation requests arrive in the
correct order. These functions use a mutex: alloc_lock.

After these functions return, the IPCP will switch to it's own
(usually read-write) lock. In the local IPCP, this leaves room for a
race where the state of the flow is accessed in alloc_resp before it
is updated in wait_flow_req_arr. This race is only visible in the
local IPCP, as the other IPCP have to send information between these
calls, but it is theoretically possible when using any IPCP for local
IPC. In the ipcpd-local, it happens ~0.01% to ~0.03% of flow allocations.

This mitigates the problem in the ipcpd-local by adding a 1ms wait to
the flow allocation if this race is detected.

Signed-off-by: Dimitri Staessens <[email protected]>
Signed-off-by: Sander Vrijders <[email protected]>

1 files changed, 15 insertions, 6 deletions

diff --git a/src/ipcpd/local/main.c b/src/ipcpd/local/main.c
index da84e517..160e07e0 100644
--- a/src/ipcpd/local/main.c
+++ b/src/ipcpd/local/main.c
@@ -221,17 +221,17 @@ static int local_ipcp_flow_alloc_resp(int              fd,
                                       int              response,
                                       const buffer_t * data)
 {
-        int    out_fd;
-        time_t mpl = IPCP_LOCAL_MPL;
+        struct timespec wait = TIMESPEC_INIT_MS(1);
+        time_t          mpl  = IPCP_LOCAL_MPL;
+        int             out_fd;
 
         if (ipcp_wait_flow_resp(fd) < 0) {
                 log_err("Failed waiting for IRMd response.");
                 return -1;
         }
 
-        pthread_rwlock_wrlock(&local_data.lock);
-
         if (response < 0) {
+                pthread_rwlock_wrlock(&local_data.lock);
                 if (local_data.in_out[fd] != -1)
                         local_data.in_out[local_data.in_out[fd]] = fd;
                 local_data.in_out[fd] = -1;
@@ -239,15 +239,24 @@ static int local_ipcp_flow_alloc_resp(int              fd,
                 return 0;
         }
 
+        pthread_rwlock_rdlock(&local_data.lock);
+
         out_fd = local_data.in_out[fd];
         if (out_fd == -1) {
                 pthread_rwlock_unlock(&local_data.lock);
-                log_err("Invalid out_fd.");
-                return -1;
+                log_dbg("Potential race detected");
+                nanosleep(&wait, NULL);
+                pthread_rwlock_rdlock(&local_data.lock);
+                out_fd = local_data.in_out[fd];
         }
 
         pthread_rwlock_unlock(&local_data.lock);
 
+        if (out_fd == -1) {
+                log_err("Invalid out_fd.");
+                return -1;
+        }
+
         fset_add(local_data.flows, fd);
 
         if (ipcp_flow_alloc_reply(out_fd, response, mpl, data) < 0) {