From: Jérémie Galarneau <jeremie.galarn...@efficios.com>

The session daemon to consumer daemon is passin around the
lttcomm_consumer_msg structure which contains a relayd_sock
structure.

This structure, in turn, contains a lttcomm_relayd_sock structure which
contains an lttcomm_sock structure.

If you're still following, this lttcomm_sock structure has an
"ops" member which is a pointer. Obviously, this pointer does not have
the same size between a 64-bit session daemon and a 32-bit consumer
which causes communication issues when adding a relayd socket (the
LTTng version is erronously interpreted as 0.2).

This fix introduces "serialized" versions of the aforementioned
structures which only use fixed-size members. Additionaly, these
"serialized" structures' members are stored as big-endian and the
rest of the communication should transition to this convention.

Signed-off-by: Jérémie Galarneau <jeremie.galarn...@efficios.com>
Signed-off-by: Yannick Lamarre <ylama...@efficios.com>
---
 src/bin/lttng-sessiond/consumer.c            |   5 +-
 src/common/kernel-consumer/kernel-consumer.c |  11 +-
 src/common/sessiond-comm/sessiond-comm.c     | 186 +++++++++++++++++++++++++++
 src/common/sessiond-comm/sessiond-comm.h     | 111 +++++++++++++++-
 src/common/ust-consumer/ust-consumer.c       |  11 +-
 tests/regression/tools/live/Makefile.am      |   2 +-
 tests/unit/Makefile.am                       |   6 +-
 7 files changed, 323 insertions(+), 9 deletions(-)

diff --git a/src/bin/lttng-sessiond/consumer.c 
b/src/bin/lttng-sessiond/consumer.c
index 80e31377..d22b0cbc 100644
--- a/src/bin/lttng-sessiond/consumer.c
+++ b/src/bin/lttng-sessiond/consumer.c
@@ -1048,7 +1048,10 @@ int consumer_send_relayd_socket(struct consumer_socket 
*consumer_sock,
        msg.u.relayd_sock.net_index = consumer->net_seq_index;
        msg.u.relayd_sock.type = type;
        msg.u.relayd_sock.session_id = session_id;
-       memcpy(&msg.u.relayd_sock.sock, rsock, sizeof(msg.u.relayd_sock.sock));
+       ret = lttcomm_relayd_sock_serialize(&msg.u.relayd_sock.sock, rsock);
+       if (ret) {
+               goto error;
+       }
 
        DBG3("Sending relayd sock info to consumer on %d", 
*consumer_sock->fd_ptr);
        ret = consumer_send_msg(consumer_sock, &msg);
diff --git a/src/common/kernel-consumer/kernel-consumer.c 
b/src/common/kernel-consumer/kernel-consumer.c
index 627cd2a8..16e3b588 100644
--- a/src/common/kernel-consumer/kernel-consumer.c
+++ b/src/common/kernel-consumer/kernel-consumer.c
@@ -454,10 +454,19 @@ int lttng_kconsumer_recv_cmd(struct 
lttng_consumer_local_data *ctx,
        switch (msg.cmd_type) {
        case LTTNG_CONSUMER_ADD_RELAYD_SOCKET:
        {
+               struct lttcomm_relayd_sock relayd_sock;
+
+               ret = lttcomm_relayd_sock_deserialize(&relayd_sock,
+                               &msg.u.relayd_sock.sock);
+               if (ret) {
+                       /* Received an invalid relayd_sock. */
+                       goto error_fatal;
+               }
+
                /* Session daemon status message are handled in the following 
call. */
                consumer_add_relayd_socket(msg.u.relayd_sock.net_index,
                                msg.u.relayd_sock.type, ctx, sock, 
consumer_sockpoll,
-                               &msg.u.relayd_sock.sock, 
msg.u.relayd_sock.session_id,
+                               &relayd_sock, msg.u.relayd_sock.session_id,
                                msg.u.relayd_sock.relayd_session_id);
                goto end_nosignal;
        }
diff --git a/src/common/sessiond-comm/sessiond-comm.c 
b/src/common/sessiond-comm/sessiond-comm.c
index 0067be16..8aee3cd1 100644
--- a/src/common/sessiond-comm/sessiond-comm.c
+++ b/src/common/sessiond-comm/sessiond-comm.c
@@ -27,6 +27,7 @@
 #include <unistd.h>
 #include <errno.h>
 #include <inttypes.h>
+#include <endian.h>
 
 #include <common/common.h>
 
@@ -75,6 +76,191 @@ static const char *lttcomm_readable_code[] = {
 
 static unsigned long network_timeout;
 
+LTTNG_HIDDEN
+int sockaddr_in_serialize(struct sockaddr_in_serialized *dst,
+               const struct sockaddr_in *src)
+{
+       assert(src && dst);
+       dst->sin_family = (uint32_t) src->sin_family;
+       dst->sin_port = (uint16_t) src->sin_port;
+       dst->sin_addr.s_addr = src->sin_addr.s_addr;
+       return 0;
+}
+
+LTTNG_HIDDEN
+int sockaddr_in_deserialize(struct sockaddr_in *dst,
+               const struct sockaddr_in_serialized *src)
+{
+       assert(src && dst);
+       dst->sin_family = (sa_family_t) src->sin_family;
+       dst->sin_port = (in_port_t) src->sin_port;
+       dst->sin_addr.s_addr = src->sin_addr.s_addr;
+       return 0;
+}
+
+LTTNG_HIDDEN
+int sockaddr_in6_serialize(struct sockaddr_in6_serialized *dst,
+               const struct sockaddr_in6 *src)
+{
+       assert(src && dst);
+
+       dst->sin6_family = (uint32_t) src->sin6_family;
+       dst->sin6_port = (uint16_t) src->sin6_port;
+       dst->sin6_flowinfo = src->sin6_flowinfo;
+       memcpy(&dst->sin6_addr._s6_addr, src->sin6_addr.s6_addr,
+                       sizeof(dst->sin6_addr._s6_addr));
+       dst->sin6_scope_id = src->sin6_scope_id;
+       return 0;
+}
+
+LTTNG_HIDDEN
+int sockaddr_in6_deserialize(struct sockaddr_in6 *dst,
+               const struct sockaddr_in6_serialized *src)
+{
+       assert(src && dst);
+
+       dst->sin6_family = (sa_family_t) src->sin6_family;
+       dst->sin6_port = (in_port_t) src->sin6_port;
+       dst->sin6_flowinfo = src->sin6_flowinfo;
+       memcpy(&dst->sin6_addr.s6_addr, src->sin6_addr._s6_addr,
+              sizeof(dst->sin6_addr.s6_addr));
+       dst->sin6_scope_id = src->sin6_scope_id;
+       return 0;
+}
+
+LTTNG_HIDDEN
+int lttcomm_sockaddr_serialize(struct lttcomm_sockaddr_serialized *dst,
+               const struct lttcomm_sockaddr *src)
+{
+       int ret = 0;
+
+       assert(src && dst);
+
+       dst->type = (uint32_t) src->type;
+
+       switch (src->type) {
+       case LTTCOMM_INET:
+       {
+               sockaddr_in_serialize(&dst->addr.sin,
+                               &src->addr.sin);
+               break;
+       }
+       case LTTCOMM_INET6:
+       {
+               sockaddr_in6_serialize(&dst->addr.sin6,
+                               &src->addr.sin6);
+               break;
+       }
+       default:
+               ret = -EINVAL;
+               break;
+       }
+
+       return ret;
+}
+
+LTTNG_HIDDEN
+int lttcomm_sockaddr_deserialize(struct lttcomm_sockaddr *dst,
+               const struct lttcomm_sockaddr_serialized *src)
+{
+       int ret = 0;
+
+       assert(src && dst);
+
+       dst->type = (enum lttcomm_sock_domain) src->type;
+
+       switch (dst->type) {
+       case LTTCOMM_INET:
+       {
+               sockaddr_in_deserialize(&dst->addr.sin,
+                               &src->addr.sin);
+               break;
+       }
+       case LTTCOMM_INET6:
+       {
+               sockaddr_in6_deserialize(&dst->addr.sin6,
+                               &src->addr.sin6);
+               break;
+       }
+       default:
+               ret = -EINVAL;
+               break;
+       }
+
+       return ret;
+}
+
+LTTNG_HIDDEN
+int lttcomm_sock_serialize(struct lttcomm_sock_serialized *dst,
+               const struct lttcomm_sock *src)
+{
+       int ret;
+
+       assert(src && dst);
+
+       dst->fd = src->fd;
+       if (src->proto != LTTCOMM_SOCK_UDP &&
+               src->proto != LTTCOMM_SOCK_TCP) {
+               /* Code flow error. */
+               assert(0);
+       }
+       dst->proto = (uint32_t) src->proto;
+       ret = lttcomm_sockaddr_serialize(&dst->sockaddr, &src->sockaddr);
+
+       return ret;
+}
+
+LTTNG_HIDDEN
+int lttcomm_sock_deserialize(struct lttcomm_sock *dst,
+               const struct lttcomm_sock_serialized *src)
+{
+       int ret;
+
+       assert(src && dst);
+
+       dst->fd = src->fd;
+       dst->proto = (enum lttcomm_sock_proto) src->proto;
+       if (dst->proto != LTTCOMM_SOCK_UDP &&
+               dst->proto != LTTCOMM_SOCK_TCP) {
+               ret = -EINVAL;
+               goto end;
+       }
+       dst->ops = NULL;
+       ret = lttcomm_sockaddr_deserialize(&dst->sockaddr, &src->sockaddr);
+
+end:
+       return ret;
+}
+
+LTTNG_HIDDEN
+int lttcomm_relayd_sock_serialize(struct lttcomm_relayd_sock_serialized *dst,
+               const struct lttcomm_relayd_sock *src)
+{
+       int ret;
+
+       assert(src && dst);
+       dst->major = src->major;
+       dst->minor = src->minor;
+       ret = lttcomm_sock_serialize(&dst->sock, &src->sock);
+
+       return ret;
+}
+
+LTTNG_HIDDEN
+int lttcomm_relayd_sock_deserialize(
+               struct lttcomm_relayd_sock *dst,
+               const struct lttcomm_relayd_sock_serialized *src)
+{
+       int ret;
+
+       assert(src && dst);
+       dst->major = src->major;
+       dst->minor = src->minor;
+       ret = lttcomm_sock_deserialize(&dst->sock, &src->sock);
+
+       return ret;
+}
+
 /*
  * Return ptr to string representing a human readable error code from the
  * lttcomm_return_code enum.
diff --git a/src/common/sessiond-comm/sessiond-comm.h 
b/src/common/sessiond-comm/sessiond-comm.h
index c0c6a8bb..556c277f 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -2,6 +2,7 @@
  * Copyright (C) 2011 - David Goulet <david.gou...@polymtl.ca>
  *                      Julien Desfossez <julien.desfos...@polymtl.ca>
  *                      Mathieu Desnoyers <mathieu.desnoy...@efficios.com>
+ *                      Jérémie Galarneau <jeremie.galarn...@efficios.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License, version 2 only,
@@ -209,31 +210,137 @@ struct lttcomm_metadata_request_msg {
        uint64_t key; /* Metadata channel key. */
 } LTTNG_PACKED;
 
+/* For internal use only */
 struct lttcomm_sockaddr {
        enum lttcomm_sock_domain type;
        union {
                struct sockaddr_in sin;
                struct sockaddr_in6 sin6;
        } addr;
+};
+
+/*
+ * Serialized version of the sockaddr_in system structure (may only be used
+ * for communication).
+ * Fields are fixed-size, big endian and packed.
+ */
+struct sockaddr_in_serialized {
+       uint32_t sin_family;
+       uint16_t sin_port;
+       struct in_addr_serialized {
+               uint32_t s_addr;
+       } LTTNG_PACKED sin_addr;
+} LTTNG_PACKED;
+
+extern
+int sockaddr_in_serialize(struct sockaddr_in_serialized *dst,
+               const struct sockaddr_in *src);
+extern
+int sockaddr_in_deserialize(struct sockaddr_in *dst,
+               const struct sockaddr_in_serialized *src);
+
+/*
+ * Serialized version of the sockaddr_in6 system structure (may only be used
+ * for communication).
+ * Fields are fixed-size, big endian and packed.
+ */
+struct sockaddr_in6_serialized {
+       uint32_t sin6_family;
+       uint16_t sin6_port;
+       uint32_t sin6_flowinfo;
+       struct in6_addr_serialized {
+               /*
+                * Prefixing with "_" since s6_addr is a "DEFINE"
+                * which clashes with this.
+                */
+               uint8_t _s6_addr[16];
+       } LTTNG_PACKED sin6_addr;
+       uint32_t sin6_scope_id;
+} LTTNG_PACKED;
+
+extern
+int sockaddr_in6_serialize(struct sockaddr_in6_serialized *dst,
+               const struct sockaddr_in6 *src);
+extern
+int sockaddr_in6_deserialize(struct sockaddr_in6 *dst,
+               const struct sockaddr_in6_serialized *src);
+
+/*
+ * Serialized version of struct lttcomm_sockaddr (may be used for
+ * communication only).
+ * The struct and its members are packed and its fields are fixed-size, big
+ * endian.
+ */
+struct lttcomm_sockaddr_serialized {
+       uint32_t type; /* Maps to enum lttcomm_sock_domain */
+       union {
+               struct sockaddr_in_serialized sin;
+               struct sockaddr_in6_serialized sin6;
+       } addr;
 } LTTNG_PACKED;
 
+extern
+int sockaddr_in6_serialize(struct sockaddr_in6_serialized *dst,
+               const struct sockaddr_in6 *src);
+extern
+int sockaddr_in6_deserialize(struct sockaddr_in6 *dst,
+               const struct sockaddr_in6_serialized *src);
+
+/* For internal use only */
 struct lttcomm_sock {
        int32_t fd;
        enum lttcomm_sock_proto proto;
        struct lttcomm_sockaddr sockaddr;
        const struct lttcomm_proto_ops *ops;
+};
+
+/*
+ * Serialized version of struct lttcomm_sock (may be used for
+ * communication only).
+ * Fields are fixed-size, big endian. Structure is packed.
+ */
+struct lttcomm_sock_serialized {
+       int32_t fd;
+       uint32_t proto; /* Maps to enum lttcomm_sock_proto */
+       struct lttcomm_sockaddr_serialized sockaddr;
 } LTTNG_PACKED;
 
+extern
+int lttcomm_sock_serialize(struct lttcomm_sock_serialized *dst,
+               const struct lttcomm_sock *src);
+extern
+int lttcomm_sock_deserialize(struct lttcomm_sock *dst,
+               const struct lttcomm_sock_serialized *src);
+
 /*
  * Relayd sock. Adds the protocol version to use for the communications with
- * the relayd.
+ * the relayd. Internal use only.
  */
 struct lttcomm_relayd_sock {
        struct lttcomm_sock sock;
        uint32_t major;
        uint32_t minor;
+};
+
+/*
+ * Serialized version of struct lttcomm_relayd_sock (may be used for
+ * communications only).
+ * Fields are fixed-size, big endian. Structure is packed.
+ */
+struct lttcomm_relayd_sock_serialized {
+       struct lttcomm_sock_serialized sock;
+       uint32_t major;
+       uint32_t minor;
 } LTTNG_PACKED;
 
+extern
+int lttcomm_relayd_sock_serialize(struct lttcomm_relayd_sock_serialized *dst,
+               const struct lttcomm_relayd_sock *src);
+extern
+int lttcomm_relayd_sock_deserialize(
+               struct lttcomm_relayd_sock *dst,
+               const struct lttcomm_relayd_sock_serialized *src);
+
 struct lttcomm_net_family {
        int family;
        int (*create) (struct lttcomm_sock *sock, int type, int proto);
@@ -486,7 +593,7 @@ struct lttcomm_consumer_msg {
                        uint64_t net_index;
                        enum lttng_stream_type type;
                        /* Open socket to the relayd */
-                       struct lttcomm_relayd_sock sock;
+                       struct lttcomm_relayd_sock_serialized sock;
                        /* Tracing session id associated to the relayd. */
                        uint64_t session_id;
                        /* Relayd session id, only used with control socket. */
diff --git a/src/common/ust-consumer/ust-consumer.c 
b/src/common/ust-consumer/ust-consumer.c
index 94b761cb..06f858c3 100644
--- a/src/common/ust-consumer/ust-consumer.c
+++ b/src/common/ust-consumer/ust-consumer.c
@@ -1354,10 +1354,19 @@ int lttng_ustconsumer_recv_cmd(struct 
lttng_consumer_local_data *ctx,
        switch (msg.cmd_type) {
        case LTTNG_CONSUMER_ADD_RELAYD_SOCKET:
        {
+               struct lttcomm_relayd_sock relayd_sock;
+
+               ret = lttcomm_relayd_sock_deserialize(&relayd_sock,
+                               &msg.u.relayd_sock.sock);
+               if (ret) {
+                       /* Received an invalid relayd_sock. */
+                       goto error_fatal;
+               }
+
                /* Session daemon status message are handled in the following 
call. */
                consumer_add_relayd_socket(msg.u.relayd_sock.net_index,
                                msg.u.relayd_sock.type, ctx, sock, 
consumer_sockpoll,
-                               &msg.u.relayd_sock.sock, 
msg.u.relayd_sock.session_id,
+                               &relayd_sock, msg.u.relayd_sock.session_id,
                                msg.u.relayd_sock.relayd_session_id);
                goto end_nosignal;
        }
diff --git a/tests/regression/tools/live/Makefile.am 
b/tests/regression/tools/live/Makefile.am
index 7a26b42c..857463e6 100644
--- a/tests/regression/tools/live/Makefile.am
+++ b/tests/regression/tools/live/Makefile.am
@@ -20,7 +20,7 @@ EXTRA_DIST += test_ust test_ust_tracefile_count test_lttng_ust
 endif
 
 live_test_SOURCES = live_test.c
-live_test_LDADD = $(LIBTAP) $(LIBCOMMON) $(LIBRELAYD) $(LIBSESSIOND_COMM) \
+live_test_LDADD = $(LIBTAP) $(LIBSESSIOND_COMM) $(LIBCOMMON) $(LIBRELAYD) \
                $(LIBHASHTABLE) $(LIBHEALTH) $(DL_LIBS) -lrt
 live_test_LDADD += $(LIVE) \
                $(top_builddir)/src/lib/lttng-ctl/liblttng-ctl.la
diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
index 5f485f00..26989fa0 100644
--- a/tests/unit/Makefile.am
+++ b/tests/unit/Makefile.am
@@ -88,7 +88,7 @@ SESSIOND_OBJS += 
$(top_builddir)/src/bin/lttng-sessiond/trace-ust.$(OBJEXT) \
 endif
 
 test_session_SOURCES = test_session.c
-test_session_LDADD = $(LIBTAP) $(LIBCOMMON) $(LIBRELAYD) $(LIBSESSIOND_COMM) \
+test_session_LDADD = $(LIBTAP) $(LIBSESSIOND_COMM) $(LIBCOMMON) $(LIBRELAYD) \
                     $(LIBHASHTABLE) $(DL_LIBS) -lrt -lurcu-common -lurcu \
                     $(KMOD_LIBS) \
                     $(top_builddir)/src/lib/lttng-ctl/liblttng-ctl.la \
@@ -108,7 +108,7 @@ endif
 # UST data structures unit test
 if HAVE_LIBLTTNG_UST_CTL
 test_ust_data_SOURCES = test_ust_data.c
-test_ust_data_LDADD = $(LIBTAP) $(LIBCOMMON) $(LIBRELAYD) $(LIBSESSIOND_COMM) \
+test_ust_data_LDADD = $(LIBTAP) $(LIBSESSIOND_COMM) $(LIBCOMMON) $(LIBRELAYD) \
                      $(LIBHASHTABLE) $(DL_LIBS) -lrt -lurcu-common -lurcu \
                      -llttng-ust-ctl \
                      $(KMOD_LIBS) \
@@ -131,7 +131,7 @@ 
KERN_DATA_TRACE=$(top_builddir)/src/bin/lttng-sessiond/trace-kernel.$(OBJEXT) \
                $(LIBLTTNG_CTL)
 
 test_kernel_data_SOURCES = test_kernel_data.c
-test_kernel_data_LDADD = $(LIBTAP) $(LIBCOMMON) $(LIBRELAYD) 
$(LIBSESSIOND_COMM) \
+test_kernel_data_LDADD = $(LIBTAP) $(LIBSESSIOND_COMM) $(LIBCOMMON) 
$(LIBRELAYD) \
                         $(LIBHASHTABLE) $(DL_LIBS) -lrt
 test_kernel_data_LDADD += $(KERN_DATA_TRACE)
 
-- 
2.11.0

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Reply via email to