pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-uecups/+/27769 )
Change subject: tun_device: Avoid deadlocks logging while thread is cancelled ......................................................................
tun_device: Avoid deadlocks logging while thread is cancelled
We cannot garantee that LOGP will not end up calling a syscall which can be a cancellation point. Since the syscall will be probably called while having the logging mutex locked, an eventuall cancellation of the thread would leave the logging mutex locked forever, hence making all other threads deadlock as soon as they try to write anything to the log.
Change-Id: I72a0b536c8f39857960f132a5b84cdf5b8519732 --- M daemon/tun_device.c 1 file changed, 11 insertions(+), 0 deletions(-)
Approvals: Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved
diff --git a/daemon/tun_device.c b/daemon/tun_device.c index 5993856..e9c0400 100644 --- a/daemon/tun_device.c +++ b/daemon/tun_device.c @@ -139,12 +139,17 @@ uint8_t *buffer = base_buffer + sizeof(struct gtp1_header);
struct sockaddr_storage daddr; + int old_cancelst_unused;
/* initialize the fixed part of the GTP header */ gtph->flags = 0x30; gtph->type = GTP_TPDU;
pthread_cleanup_push(tun_device_pthread_cleanup_routine, tun); + /* IMPORTANT!: All logging functions in this function block must be called with + * PTHREAD_CANCEL_DISABLE set, otherwise the thread could be cancelled while + * holding the logging mutex, hence causing deadlock with main (or other) + * thread. */
while (1) { struct gtp_tunnel *t; @@ -154,6 +159,7 @@ /* 1) read from tun */ rc = read(tun->fd, buffer, MAX_UDP_PACKET); if (rc < 0) { + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelst_unused); LOGTUN(tun, LOGL_FATAL, "Error readingfrom tun device: %s\n", strerror(errno)); exit(1); } @@ -162,8 +168,10 @@
rc = parse_pkt(&pinfo, buffer, nread); if (rc < 0) { + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelst_unused); LOGTUN(tun, LOGL_NOTICE, "Error parsing IP packet: %s\n", osmo_hexdump(buffer, nread)); + pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_cancelst_unused); continue; }
@@ -181,7 +189,9 @@ getnameinfo((const struct sockaddr *)&pinfo.saddr, sizeof(pinfo.saddr), host, sizeof(host), port, sizeof(port), NI_NUMERICHOST | NI_NUMERICSERV); + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelst_unused); LOGTUN(tun, LOGL_NOTICE, "No tunnel found for source address %s:%s\n", host, port); + pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_cancelst_unused); continue; } outfd = t->gtp_ep->fd; @@ -193,6 +203,7 @@ rc = sendto(outfd, base_buffer, nread+sizeof(*gtph), 0, (struct sockaddr *)&daddr, sizeof(daddr)); if (rc < 0) { + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelst_unused); LOGTUN(tun, LOGL_FATAL, "Error Writing to UDP socket: %s\n", strerror(errno)); exit(1); }