pespin has uploaded this change for review.

View Change

gtp_endpoint: 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 eventual 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.

Similar to what's already done in tun_device_thread().

Change-Id: I1e141175440402a7db34f3d246104c6ea38031fb
---
M daemon/gtp_endpoint.c
1 file changed, 23 insertions(+), 2 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/osmo-uecups refs/changes/88/40788/1
diff --git a/daemon/gtp_endpoint.c b/daemon/gtp_endpoint.c
index bb417da..08c0f44 100644
--- a/daemon/gtp_endpoint.c
+++ b/daemon/gtp_endpoint.c
@@ -36,27 +36,36 @@
int rc, outfd;
uint32_t teid;
uint16_t gtp_len;
+ int old_cancelst_unused;

if (nread < sizeof(*gtph)) {
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelst_unused);
LOGEP(ep, LOGL_NOTICE, "Short read: %u < %lu\n", nread, sizeof(*gtph));
+ pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_cancelst_unused);
return;
}
gtph = (struct gtp1_header *)buffer;

/* check GTP header contents */
if ((gtph->flags & 0xf0) != 0x30) {
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelst_unused);
LOGEP(ep, LOGL_NOTICE, "Unexpected GTP Flags: 0x%02x\n", gtph->flags);
+ pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_cancelst_unused);
return;
}
if (gtph->type != GTP_TPDU) {
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelst_unused);
LOGEP(ep, LOGL_NOTICE, "Unexpected GTP Message Type: 0x%02x\n", gtph->type);
+ pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_cancelst_unused);
return;
}

gtp_len = ntohs(gtph->length);
if (sizeof(*gtph)+gtp_len > nread) {
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelst_unused);
LOGEP(ep, LOGL_NOTICE, "Short GTP Message: %lu < len=%u\n",
sizeof(*gtph)+gtp_len, nread);
+ pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_cancelst_unused);
return;
}
teid = ntohl(gtph->tid);
@@ -65,8 +74,10 @@
if (gtph->flags & GTP1_F_MASK) {
const struct gtp1_exthdr *exthdr = (const struct gtp1_exthdr *)payload;
if (gtp_len < 4) {
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelst_unused);
LOGEP(ep, LOGL_NOTICE, "Short GTP Message according to flags 0x%02x: %lu < len=%u\n",
gtph->flags, sizeof(*gtph) + gtp_len, nread);
+ pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_cancelst_unused);
return;
}
gtp_len -= 4;
@@ -77,12 +88,14 @@
if (gtp_len < 1) {
LOGEP(ep, LOGL_NOTICE, "Short GTP Message according to flags 0x%02x: %lu < len=%u\n",
gtph->flags, sizeof(*gtph) + gtp_len, nread);
+ pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_cancelst_unused);
return;
}
ext_len = 1 + 1 + it[1] + 1;
if (gtp_len < ext_len) {
LOGEP(ep, LOGL_NOTICE, "Short GTP Message according to flags 0x%02x: %lu < len=%u\n",
gtph->flags, sizeof(*gtph) + gtp_len, nread);
+ pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_cancelst_unused);
return;
}
gtp_len -= ext_len;
@@ -96,7 +109,9 @@
t = _gtp_tunnel_find_r(d, teid, ep);
if (!t) {
pthread_rwlock_unlock(&d->rwlock);
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelst_unused);
LOGEP(ep, LOGL_NOTICE, "Unable to find tunnel for TEID=0x%08x\n", teid);
+ pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_cancelst_unused);
return;
}
outfd = t->tun_dev->fd;
@@ -105,16 +120,21 @@
/* 3) write to TUN device */
rc = write(outfd, payload, gtp_len);
if (rc < gtp_len) {
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelst_unused);
LOGEP(ep, LOGL_FATAL, "Error writing to tun device %s\n", strerror(errno));
exit(1);
}
}

-/* one thread for reading from each GTP/UDP socket (GTP decapsulation -> tun) */
+/* one thread for reading from each GTP/UDP socket (GTP decapsulation -> 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. */
static void *gtp_endpoint_thread(void *arg)
{
struct gtp_endpoint *ep = (struct gtp_endpoint *)arg;
-
+ int old_cancelst_unused;
uint8_t buffer[sizeof(struct gtp1_header) + sizeof(struct gtp1_exthdr) + MAX_UDP_PACKET];

while (1) {
@@ -123,6 +143,7 @@
/* 1) read GTP packet from UDP socket */
rc = recvfrom(ep->fd, buffer, sizeof(buffer), 0, (struct sockaddr *)NULL, 0);
if (rc < 0) {
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelst_unused);
LOGEP(ep, LOGL_FATAL, "Error reading from UDP socket: %s\n", strerror(errno));
exit(1);
}

To view, visit change 40788. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: newchange
Gerrit-Project: osmo-uecups
Gerrit-Branch: master
Gerrit-Change-Id: I1e141175440402a7db34f3d246104c6ea38031fb
Gerrit-Change-Number: 40788
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin@sysmocom.de>