Skip to content

Commit

Permalink
Fix SEGFAULT in BMI when removing a port
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
antoninbas committed Mar 2, 2019
1 parent 942a9ba commit 5d25d0d
Showing 1 changed file with 24 additions and 2 deletions.
26 changes: 24 additions & 2 deletions src/BMI/bmi_port.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
#include "BMI/bmi_port.h"

#include <fcntl.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/types.h>

typedef struct bmi_port_s {
bmi_interface_t *bmi;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit 5d25d0d

Please sign in to comment.