[lttng-dev] [RFC PATCH lttng-tools 01/18] Fix: Use platform-independant types in sessiond to consumerd communication
Yannick Lamarre
ylamarre at efficios.com
Thu Apr 18 12:18:33 EDT 2019
From: Jérémie Galarneau <jeremie.galarneau at 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.galarneau at efficios.com>
Signed-off-by: Yannick Lamarre <ylamarre at 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.goulet at polymtl.ca>
* Julien Desfossez <julien.desfossez at polymtl.ca>
* Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
+ * Jérémie Galarneau <jeremie.galarneau at 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
More information about the lttng-dev
mailing list