summaryrefslogtreecommitdiff
path: root/src/ipcpd
diff options
context:
space:
mode:
authorDimitri Staessens <dimitri@ouroboros.rocks>2024-02-24 12:35:22 +0100
committerSander Vrijders <sander@ouroboros.rocks>2024-02-26 08:59:15 +0100
commitb5888ad746fd03d483196a2cb3711bb0fe63d82d (patch)
tree5de0df7212b7f6d0050431ec92b59f42aa48a3b3 /src/ipcpd
parentdc56c2211424567d9a249f3630505d92f026428e (diff)
downloadouroboros-b5888ad746fd03d483196a2cb3711bb0fe63d82d.tar.gz
ouroboros-b5888ad746fd03d483196a2cb3711bb0fe63d82d.zip
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 <dimitri@ouroboros.rocks> Signed-off-by: Sander Vrijders <sander@ouroboros.rocks>
Diffstat (limited to 'src/ipcpd')
-rw-r--r--src/ipcpd/local/main.c21
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) {