From 5d25d0d94681492d155d3e5b72b16a56121f8dfe Mon Sep 17 00:00:00 2001 From: Antonin Bas Date: Fri, 1 Mar 2019 16:23:25 -0800 Subject: [PATCH] Fix SEGFAULT in BMI when removing a port There were 2 issues with the code: * The FD_CLR call had been moved after the memset of the bmi_port_t structure, which means that we were not clearing the file descriptor at all. * We should not close the file descriptor while the select call is on-going, it is undefined behavior. We introduce a socketpair so that the thread removing the port can notify the select thread and wait for an acknowledgement before closing the file descriptor. --- src/BMI/bmi_port.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/BMI/bmi_port.c b/src/BMI/bmi_port.c index 682ce4060..7a278c5f0 100644 --- a/src/BMI/bmi_port.c +++ b/src/BMI/bmi_port.c @@ -30,7 +30,9 @@ #include "BMI/bmi_port.h" #include +#include #include +#include typedef struct bmi_port_s { bmi_interface_t *bmi; @@ -45,6 +47,7 @@ typedef struct bmi_port_s { typedef struct bmi_port_mgr_s { bmi_port_t ports_info[PORT_COUNT_MAX]; + int socketpairfd[2]; fd_set fds; int max_fd; void *cookie; @@ -98,6 +101,12 @@ static void *run_select(void *data) { /* the thread terminates */ if(max_fd == -1) return NULL; + if (FD_ISSET(port_mgr->socketpairfd[1], &fds)) { + char buf[1] = {'\x00'}; + read(port_mgr->socketpairfd[1], buf, sizeof(buf)); + write(port_mgr->socketpairfd[1], buf, sizeof(buf)); + } + if(n <= 0) { // timeout or EINTR continue; } @@ -138,7 +147,13 @@ int bmi_port_create_mgr(bmi_port_mgr_t **port_mgr) { memset(port_mgr_, 0, sizeof(bmi_port_mgr_t)); + if (socketpair(PF_LOCAL, SOCK_STREAM, 0, port_mgr_->socketpairfd)) { + perror("socketpair"); + return -1; + } + FD_ZERO(&port_mgr_->fds); + FD_SET(port_mgr_->socketpairfd[1], &port_mgr_->fds); exitCode = pthread_rwlock_init(&port_mgr_->lock, NULL); if (exitCode != 0) @@ -237,12 +252,16 @@ static int _bmi_port_interface_remove(bmi_port_mgr_t *port_mgr, int port_num) { if(!port_in_use(port)) return -1; free(port->ifname); + FD_CLR(port->fd, &port_mgr->fds); + + char buf[1] = {'\x00'}; + write(port_mgr->socketpairfd[0], buf, sizeof(buf)); + read(port_mgr->socketpairfd[0], buf, sizeof(buf)); + if(bmi_interface_destroy(port->bmi) != 0) return -1; memset(port, 0, sizeof(bmi_port_t)); - FD_CLR(port->fd, &port_mgr->fds); - return 0; } @@ -272,6 +291,9 @@ int bmi_port_destroy_mgr(bmi_port_mgr_t *port_mgr) { pthread_mutex_destroy(&port->stats_lock); } + close(port_mgr->socketpairfd[0]); + close(port_mgr->socketpairfd[1]); + pthread_rwlock_destroy(&port_mgr->lock); free(port_mgr);