From f6afe3602b2a595af547ec190a1fa76cae8d2e87 Mon Sep 17 00:00:00 2001 From: illiliti Date: Mon, 17 Aug 2020 16:57:50 +0300 Subject: [PATCH] eliminate race conditions --- udev_monitor.c | 105 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 72 insertions(+), 33 deletions(-) diff --git a/udev_monitor.c b/udev_monitor.c index 0b44766..c434ad8 100644 --- a/udev_monitor.c +++ b/udev_monitor.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -23,9 +24,11 @@ struct udev_monitor { struct udev_list_entry subsystem_match; struct udev_list_entry devtype_match; pthread_t thread[THREADS_MAX]; + pthread_barrier_t barrier; struct udev *udev; int refcount; int sfd[2]; + int pfd[2]; int ifd; int efd; }; @@ -113,16 +116,26 @@ static void *udev_monitor_handle_event(void *ptr) { struct udev_monitor *udev_monitor = ptr; struct inotify_event *event; - struct epoll_event epoll; + struct epoll_event epoll[2]; char data[4096]; sigset_t mask; ssize_t len; int i; - sigemptyset(&mask); - sigaddset(&mask, SIGUSR1); + sigfillset(&mask); + pthread_sigmask(SIG_BLOCK, &mask, NULL); + + while (epoll_wait(udev_monitor->efd, epoll, 2, -1) != -1) { + for (i = 0; i < 2; i++) { + if (epoll[i].data.fd == udev_monitor->pfd[0]) { + read(udev_monitor->pfd[0], data, sizeof(data)); + write(udev_monitor->pfd[1], "1", 1); + + pthread_barrier_wait(&udev_monitor->barrier); + return NULL; + } + } - while (epoll_pwait(udev_monitor->efd, &epoll, 1, -1, &mask) != -1) { len = read(udev_monitor->ifd, data, sizeof(data)); if (len == -1) { @@ -135,6 +148,7 @@ static void *udev_monitor_handle_event(void *ptr) } } + pthread_barrier_wait(&udev_monitor->barrier); return NULL; } @@ -149,6 +163,7 @@ int udev_monitor_enable_receiving(struct udev_monitor *udev_monitor) pthread_attr_init(&attr); pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); + pthread_barrier_init(&udev_monitor->barrier, NULL, THREADS_MAX + 1); for (i = 0; i < THREADS_MAX; i++) { pthread_create(&udev_monitor->thread[i], &attr, udev_monitor_handle_event, udev_monitor); @@ -210,8 +225,9 @@ int udev_monitor_filter_add_match_tag(struct udev_monitor *udev_monitor, const c struct udev_monitor *udev_monitor_new_from_netlink(struct udev *udev, const char *name) { struct udev_monitor *udev_monitor; - struct epoll_event epoll; + struct epoll_event epoll[2]; struct stat st; + int i; if (!udev || !name) { return NULL; @@ -224,12 +240,8 @@ struct udev_monitor *udev_monitor_new_from_netlink(struct udev *udev, const char } if (lstat(UDEV_MONITOR_DIR, &st) != 0) { - if (mkdir(UDEV_MONITOR_DIR, 0) == -1) { - free(udev_monitor); - return NULL; - } - - if (chmod(UDEV_MONITOR_DIR, 0777) == -1) { + if (mkdir(UDEV_MONITOR_DIR, 0) == -1 || + chmod(UDEV_MONITOR_DIR, 0777) == -1) { free(udev_monitor); return NULL; } @@ -239,37 +251,63 @@ struct udev_monitor *udev_monitor_new_from_netlink(struct udev *udev, const char return NULL; } - udev_monitor->ifd = inotify_init1(IN_CLOEXEC | IN_NONBLOCK); - - if (udev_monitor->ifd == -1) { - free(udev_monitor); - return NULL; - } - - if (inotify_add_watch(udev_monitor->ifd, UDEV_MONITOR_DIR, IN_CREATE) == -1) { - close(udev_monitor->ifd); - free(udev_monitor); - return NULL; - } - udev_monitor->efd = epoll_create1(EPOLL_CLOEXEC); if (udev_monitor->efd == -1) { - close(udev_monitor->ifd); free(udev_monitor); return NULL; } - epoll.events = EPOLLIN | EPOLLET; + udev_monitor->ifd = inotify_init1(IN_CLOEXEC | IN_NONBLOCK); - if (epoll_ctl(udev_monitor->efd, EPOLL_CTL_ADD, udev_monitor->ifd, &epoll) == -1) { + if (udev_monitor->ifd == -1) { + close(udev_monitor->efd); + free(udev_monitor); + return NULL; + } + + if (inotify_add_watch(udev_monitor->ifd, UDEV_MONITOR_DIR, IN_CLOSE_WRITE) == -1) { close(udev_monitor->ifd); close(udev_monitor->efd); free(udev_monitor); return NULL; } - if (socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, udev_monitor->sfd) == -1) { + if (pipe(udev_monitor->pfd) == -1) { + close(udev_monitor->ifd); + close(udev_monitor->efd); + free(udev_monitor); + return NULL; + } + + for (i = 0; i < 2; i++) { + fcntl(udev_monitor->pfd[i], F_SETFD, FD_CLOEXEC); + fcntl(udev_monitor->pfd[i], F_SETFL, O_NONBLOCK); + } + + for (i = 0; i < 2; i++) { + epoll[i].events = EPOLLIN | EPOLLET; + } + + epoll[1].data.fd = udev_monitor->pfd[0]; + + if (epoll_ctl(udev_monitor->efd, EPOLL_CTL_ADD, udev_monitor->ifd, &epoll[0]) == -1 || + epoll_ctl(udev_monitor->efd, EPOLL_CTL_ADD, udev_monitor->pfd[0], &epoll[1]) == -1) { + for (i = 0; i < 2; i++) { + close(udev_monitor->pfd[i]); + } + + close(udev_monitor->ifd); + close(udev_monitor->efd); + free(udev_monitor); + return NULL; + } + + if (socketpair(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, udev_monitor->sfd) == -1) { + for (i = 0; i < 2; i++) { + close(udev_monitor->pfd[i]); + } + close(udev_monitor->ifd); close(udev_monitor->efd); free(udev_monitor); @@ -303,16 +341,17 @@ struct udev_monitor *udev_monitor_unref(struct udev_monitor *udev_monitor) return NULL; } - udev_list_entry_free_all(&udev_monitor->devtype_match); udev_list_entry_free_all(&udev_monitor->subsystem_match); + udev_list_entry_free_all(&udev_monitor->devtype_match); - // TODO fix possible race condition and UB - for (i = 0; i < THREADS_MAX; i++) { - pthread_kill(udev_monitor->thread[i], SIGUSR1); - } + write(udev_monitor->pfd[1], "1", 1); + + pthread_barrier_wait(&udev_monitor->barrier); + pthread_barrier_destroy(&udev_monitor->barrier); for (i = 0; i < 2; i++) { close(udev_monitor->sfd[i]); + close(udev_monitor->pfd[i]); } close(udev_monitor->ifd);