Fix: Unexpected payload size in cmd_recv_stream_2_11
authorJonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Tue, 11 Jan 2022 17:28:15 +0000 (12:28 -0500)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Tue, 22 Feb 2022 23:45:56 +0000 (18:45 -0500)
Observed issue
==============

For the following scenario:

 lttng-relayd: 64 bit
 lttng-sessiond: 64 bit
 lttng-consumerd: 32 bit
 application: 32 bit

 Commands
   lttng create --set-url=net://127.0.0.1
   lttng enable-event -u -a
   lttng start
   ./application

On application start the lttng-relayd reports this error:

  DEBUG1 - 14:16:38.216442600 [2004731/2004735]: Done receiving control command payload: fd = 19, payload size = 4376 bytes (in relay_process_control_receive_payload() at main.c:3456)
  DEBUG3 - 14:16:38.216469462 [2004731/2004735]: Processing "RELAYD_ADD_STREAM" command for socket 19 (in relay_process_control_command() at main.c:3327)
  Error: Unexpected payload size in "cmd_recv_stream_2_11": expected >= 3519925694 bytes, got 4376 bytes

Cause
=====

In `relayd_add_stream`, instead of taking the > 2.11 protocol path, the
`relayd_add_stream_2_2` function is called.

The value of the rsock version number are:

  major: 21845
  minor: 2

Which is simply invalid since we know that the version should be 2.12.

The relayd sock version numbers are set during the
LTTNG_CONSUMER_ADD_RELAYD_SOCKET command between the lttng-sessiond and
the lttng-consumerd process. It is important to note here that both
processes do NOT have the same bitness.

The serialization and deserialization of `struct lttcomm_relayd_sock` is
the culprit.

`struct lttcomm_relayd_sock` contains a `struct lttcomm_sock`:

struct lttcomm_sock {
    int32_t fd;
    enum lttcomm_sock_proto proto;
    struct lttcomm_sockaddr sockaddr;
    const struct lttcomm_proto_ops *ops;
} LTTNG_PACKED;

Note that `ops` is a pointer and its size varies based on the bitness of
the application. Hence the size of the `struct lttcomm_sock` differs
across bitness. Since it is the first member of `struct
lttcomm_relayd_sock`, the memory layout is simply invalid across
bitness (amd64/x86).

This results in invalid parsing for the overall "struct
lttcomm_relayd_sock" when dealing with a lttng-consumerd with a
different bitness than the lttng-sessiond. As far as I know local
tracing scenarios are not affected since this is only relevant when
dealing with a lttng-relayd.

Solution
========

Pass the socket protocol type, relayd major, relayd minor in
`lttcomm_consumer_msg`. On the receiver side, query the network stack to
get the peer information to populate a basic `lttcomm_sock`. Leaving
this work to the OS saves us from having to serialize the `sockaddr_in*`
structs.

Known drawbacks
=========

We rely on `getpeername` for the first time. Compatibility might be a
problem.

This code path assumes a lot of thing that cannot be asserted against
such as the fact that the socket from which we fetch the info must be
`connected`. Still at this point, the socket is completely setup and the
rest of the code depends on it already.

From GETPEERNAME(2):

```
       For stream sockets, once a connect(2) has been performed, either
       socket can call getpeername() to obtain the address of the peer
       socket.  On the other hand, datagram sockets  are connectionless.
       Calling connect(2) on a datagram socket merely sets the peer
       address for outgoing datagrams sent with write(2) or recv(2).
       The caller of connect(2) can use getpeername() to obtain the
       peer address that it earlier set for the socket.  However, the
       peer socket is unaware of this information, and calling
       getpeername() on the  peer  socket will  return  no useful
       information (unless a connect(2) call was also executed on the
       peer).  Note also that the receiver of a datagram can obtain the
       address of the sender when using recvfrom(2).
```

But here we are always "the caller of connect".

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ic157c4137b2f20e394c907136687fcbd126f90a0

src/bin/lttng-sessiond/consumer.c
src/common/consumer/consumer.c
src/common/consumer/consumer.h
src/common/kernel-consumer/kernel-consumer.c
src/common/sessiond-comm/sessiond-comm.c
src/common/sessiond-comm/sessiond-comm.h
src/common/ust-consumer/ust-consumer.c
src/lib/lttng-ctl/deprecated-symbols.c

index 4ba40a075fdae27516c53406f6cc1a1e9938f45d..718b57e53e87d14e05ce2c46ffb8d0f94fc97bb2 100644 (file)
@@ -1179,7 +1179,9 @@ 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));
+       msg.u.relayd_sock.major = rsock->major;
+       msg.u.relayd_sock.minor = rsock->minor;
+       msg.u.relayd_sock.relayd_socket_protocol = rsock->sock.proto;
 
        DBG3("Sending relayd sock info to consumer on %d", *consumer_sock->fd_ptr);
        ret = consumer_send_msg(consumer_sock, &msg);
index e37e9d4687ed2bcf4afad4d206fa4a886d1b5c28..301a8bc7ac52d3574c89ff73d5fa975ae75001a6 100644 (file)
@@ -8,6 +8,7 @@
  */
 
 #include "common/index/ctf-index.h"
+#include <stdint.h>
 #define _LGPL_SOURCE
 #include <assert.h>
 #include <poll.h>
@@ -3563,18 +3564,22 @@ error:
  * This will create a relayd socket pair and add it to the relayd hash table.
  * The caller MUST acquire a RCU read side lock before calling it.
  */
- void consumer_add_relayd_socket(uint64_t net_seq_idx, int sock_type,
-               struct lttng_consumer_local_data *ctx, int sock,
+void consumer_add_relayd_socket(uint64_t net_seq_idx,
+               int sock_type,
+               struct lttng_consumer_local_data *ctx,
+               int sock,
                struct pollfd *consumer_sockpoll,
-               struct lttcomm_relayd_sock *relayd_sock, uint64_t sessiond_id,
-               uint64_t relayd_session_id)
+               uint64_t sessiond_id,
+               uint64_t relayd_session_id,
+               uint32_t relayd_version_major,
+               uint32_t relayd_version_minor,
+               enum lttcomm_sock_proto relayd_socket_protocol)
 {
        int fd = -1, ret = -1, relayd_created = 0;
        enum lttcomm_return_code ret_code = LTTCOMM_CONSUMERD_SUCCESS;
        struct consumer_relayd_sock_pair *relayd = NULL;
 
        assert(ctx);
-       assert(relayd_sock);
 
        DBG("Consumer adding relayd socket (idx: %" PRIu64 ")", net_seq_idx);
 
@@ -3643,54 +3648,25 @@ error:
        switch (sock_type) {
        case LTTNG_STREAM_CONTROL:
                /* Copy received lttcomm socket */
-               lttcomm_copy_sock(&relayd->control_sock.sock, &relayd_sock->sock);
-               ret = lttcomm_create_sock(&relayd->control_sock.sock);
-               /* Handle create_sock error. */
-               if (ret < 0) {
-                       ret_code = LTTCOMM_CONSUMERD_ENOMEM;
-                       goto error;
-               }
-               /*
-                * Close the socket created internally by
-                * lttcomm_create_sock, so we can replace it by the one
-                * received from sessiond.
-                */
-               if (close(relayd->control_sock.sock.fd)) {
-                       PERROR("close");
-               }
+               ret = lttcomm_populate_sock_from_open_socket(
+                               &relayd->control_sock.sock, fd,
+                               relayd_socket_protocol);
 
-               /* Assign new file descriptor */
-               relayd->control_sock.sock.fd = fd;
                /* Assign version values. */
-               relayd->control_sock.major = relayd_sock->major;
-               relayd->control_sock.minor = relayd_sock->minor;
+               relayd->control_sock.major = relayd_version_major;
+               relayd->control_sock.minor = relayd_version_minor;
 
                relayd->relayd_session_id = relayd_session_id;
 
                break;
        case LTTNG_STREAM_DATA:
                /* Copy received lttcomm socket */
-               lttcomm_copy_sock(&relayd->data_sock.sock, &relayd_sock->sock);
-               ret = lttcomm_create_sock(&relayd->data_sock.sock);
-               /* Handle create_sock error. */
-               if (ret < 0) {
-                       ret_code = LTTCOMM_CONSUMERD_ENOMEM;
-                       goto error;
-               }
-               /*
-                * Close the socket created internally by
-                * lttcomm_create_sock, so we can replace it by the one
-                * received from sessiond.
-                */
-               if (close(relayd->data_sock.sock.fd)) {
-                       PERROR("close");
-               }
-
-               /* Assign new file descriptor */
-               relayd->data_sock.sock.fd = fd;
+               ret = lttcomm_populate_sock_from_open_socket(
+                               &relayd->data_sock.sock, fd,
+                               relayd_socket_protocol);
                /* Assign version values. */
-               relayd->data_sock.major = relayd_sock->major;
-               relayd->data_sock.minor = relayd_sock->minor;
+               relayd->data_sock.major = relayd_version_major;
+               relayd->data_sock.minor = relayd_version_minor;
                break;
        default:
                ERR("Unknown relayd socket type (%d)", sock_type);
@@ -3698,6 +3674,11 @@ error:
                goto error;
        }
 
+       if (ret < 0) {
+               ret_code = LTTCOMM_CONSUMERD_FATAL;
+               goto error;
+       }
+
        DBG("Consumer %s socket created successfully with net idx %" PRIu64 " (fd: %d)",
                        sock_type == LTTNG_STREAM_CONTROL ? "control" : "data",
                        relayd->net_seq_idx, fd);
index c3e15e3163fb912e6da41ecba92c51434b37b864..0aaa73e5550e83c5a6fc6cab189198beea4de8a5 100644 (file)
@@ -13,6 +13,7 @@
 
 #include <limits.h>
 #include <poll.h>
+#include <stdint.h>
 #include <unistd.h>
 #include <urcu/list.h>
 
@@ -1014,10 +1015,16 @@ ssize_t lttng_consumer_read_subbuffer(struct lttng_consumer_stream *stream,
                struct lttng_consumer_local_data *ctx,
                bool locked_by_caller);
 int lttng_consumer_on_recv_stream(struct lttng_consumer_stream *stream);
-void consumer_add_relayd_socket(uint64_t net_seq_idx, int sock_type,
-               struct lttng_consumer_local_data *ctx, int sock,
-               struct pollfd *consumer_sockpoll, struct lttcomm_relayd_sock *relayd_sock,
-               uint64_t sessiond_id, uint64_t relayd_session_id);
+void consumer_add_relayd_socket(uint64_t net_seq_idx,
+               int sock_type,
+               struct lttng_consumer_local_data *ctx,
+               int sock,
+               struct pollfd *consumer_sockpoll,
+               uint64_t sessiond_id,
+               uint64_t relayd_session_id,
+               uint32_t relayd_version_major,
+               uint32_t relayd_version_minor,
+               enum lttcomm_sock_proto relayd_socket_protocol);
 void consumer_flag_relayd_for_destroy(
                struct consumer_relayd_sock_pair *relayd);
 int consumer_data_pending(uint64_t id);
index b6440bc2aef018102e48dcac2fa20c53b33f86db..423b3f7bb2905196d78f9265edf98677dc9cf12d 100644 (file)
@@ -484,11 +484,17 @@ int lttng_kconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
        switch (msg.cmd_type) {
        case LTTNG_CONSUMER_ADD_RELAYD_SOCKET:
        {
+               uint32_t major = msg.u.relayd_sock.major;
+               uint32_t minor = msg.u.relayd_sock.minor;
+               enum lttcomm_sock_proto protocol = (enum lttcomm_sock_proto)
+                               msg.u.relayd_sock.relayd_socket_protocol;
+
                /* 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,
-                               msg.u.relayd_sock.relayd_session_id);
+                               msg.u.relayd_sock.type, ctx, sock,
+                               consumer_sockpoll, msg.u.relayd_sock.session_id,
+                               msg.u.relayd_sock.relayd_session_id, major,
+                               minor, protocol);
                goto end_nosignal;
        }
        case LTTNG_CONSUMER_ADD_CHANNEL:
index 5f0cd0140ffd745d2c4d16bb7c155e6d94a131cc..d64374ea9e30bd330c73763b80260b0359bb89cf 100644 (file)
@@ -6,6 +6,7 @@
  *
  */
 
+#include <sys/socket.h>
 #define _LGPL_SOURCE
 #include <assert.h>
 #include <limits.h>
@@ -477,3 +478,72 @@ unsigned long lttcomm_get_network_timeout(void)
 {
        return network_timeout;
 }
+
+/*
+ * Only valid for an ipv4 and ipv6 bound socket that is already connected to its
+ * peer.
+ */
+int lttcomm_populate_sock_from_open_socket(
+               struct lttcomm_sock *sock,
+               int fd,
+               enum lttcomm_sock_proto protocol)
+{
+       int ret = 0;
+       socklen_t storage_len;
+       struct sockaddr_storage storage = { 0 };
+
+       assert(sock);
+       assert(fd >= 0);
+
+       sock->proto = protocol;
+
+       storage_len = sizeof(storage);
+       ret = getpeername(fd, (struct sockaddr *) &storage,
+                       &storage_len);
+       if (ret) {
+               ERR("Failed to get peer info for socket %d (errno: %d)", fd,
+                               errno);
+               ret = -1;
+               goto end;
+       }
+
+       if (storage_len > sizeof(storage)) {
+               ERR("Failed to get peer info for socket %d: storage size is too small", fd);
+               ret = -1;
+               goto end;
+       }
+
+       switch (storage.ss_family) {
+       case AF_INET:
+               sock->sockaddr.type = LTTCOMM_INET;
+               memcpy(&sock->sockaddr.addr, &storage,
+                               sizeof(struct sockaddr_in));
+               break;
+       case AF_INET6:
+               sock->sockaddr.type = LTTCOMM_INET6;
+               memcpy(&sock->sockaddr.addr, &storage,
+                               sizeof(struct sockaddr_in6));
+               break;
+       default:
+               abort();
+               break;
+       }
+
+       /* Create a valid socket object with a temporary fd. */
+       ret = lttcomm_create_sock(sock);
+       if (ret < 0) {
+               ERR("Failed to create temporary socket object");
+               ret = -1;
+               goto end;
+       }
+
+       /* Substitute the fd. */
+       if (sock->ops->close(sock)) {
+               ret = -1;
+               goto end;
+       }
+       sock->fd = fd;
+
+end:
+       return ret;
+}
index e23a84d20ee2d3feccecb79b543eb0ee88a0c01d..2d6493d8eec989b53c17e361b8f5665bae0f9bb8 100644 (file)
@@ -30,6 +30,7 @@
 
 #include <arpa/inet.h>
 #include <netinet/in.h>
+#include <stdint.h>
 #include <sys/un.h>
 
 #include "inet.h"
@@ -325,14 +326,14 @@ struct lttcomm_sockaddr {
                struct sockaddr_in sin;
                struct sockaddr_in6 sin6;
        } addr;
-} LTTNG_PACKED;
+};
 
 struct lttcomm_sock {
        int32_t fd;
        enum lttcomm_sock_proto proto;
        struct lttcomm_sockaddr sockaddr;
        const struct lttcomm_proto_ops *ops;
-} LTTNG_PACKED;
+};
 
 /*
  * Relayd sock. Adds the protocol version to use for the communications with
@@ -342,7 +343,7 @@ struct lttcomm_relayd_sock {
        struct lttcomm_sock sock;
        uint32_t major;
        uint32_t minor;
-} LTTNG_PACKED;
+};
 
 struct lttcomm_net_family {
        int family;
@@ -657,8 +658,9 @@ struct lttcomm_consumer_msg {
                struct {
                        uint64_t net_index;
                        enum lttng_stream_type type;
-                       /* Open socket to the relayd */
-                       struct lttcomm_relayd_sock sock;
+                       uint32_t major;
+                       uint32_t minor;
+                       uint8_t relayd_socket_protocol;
                        /* Tracing session id associated to the relayd. */
                        uint64_t session_id;
                        /* Relayd session id, only used with control socket. */
@@ -904,6 +906,9 @@ LTTNG_HIDDEN int lttcomm_init_inet6_sockaddr(struct lttcomm_sockaddr *sockaddr,
                const char *ip, unsigned int port);
 
 LTTNG_HIDDEN struct lttcomm_sock *lttcomm_alloc_sock(enum lttcomm_sock_proto proto);
+LTTNG_HIDDEN int lttcomm_populate_sock_from_open_socket(struct lttcomm_sock *sock,
+               int fd,
+               enum lttcomm_sock_proto protocol);
 LTTNG_HIDDEN int lttcomm_create_sock(struct lttcomm_sock *sock);
 LTTNG_HIDDEN struct lttcomm_sock *lttcomm_alloc_sock_from_uri(struct lttng_uri *uri);
 LTTNG_HIDDEN void lttcomm_destroy_sock(struct lttcomm_sock *sock);
index fa1c71299d2469fbfb2a0579fc844d1ded8de8af..54ad208c0c7dbb90f303d5d8269a3fde21590a38 100644 (file)
@@ -1428,11 +1428,18 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
        switch (msg.cmd_type) {
        case LTTNG_CONSUMER_ADD_RELAYD_SOCKET:
        {
+               uint32_t major = msg.u.relayd_sock.major;
+               uint32_t minor = msg.u.relayd_sock.minor;
+               enum lttcomm_sock_proto protocol =
+                               (enum lttcomm_sock_proto) msg.u.relayd_sock
+                                               .relayd_socket_protocol;
+
                /* 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,
-                               msg.u.relayd_sock.relayd_session_id);
+                               msg.u.relayd_sock.type, ctx, sock,
+                               consumer_sockpoll, msg.u.relayd_sock.session_id,
+                               msg.u.relayd_sock.relayd_session_id, major,
+                               minor, protocol);
                goto end_nosignal;
        }
        case LTTNG_CONSUMER_DESTROY_RELAYD:
index 8b1a8e7a9629938311e041ee14e96f8bd2ea7003..15e74d77af141e96ef703e330687b3b0921d844c 100644 (file)
@@ -24,28 +24,28 @@ const char * const config_element_targets;
 const char * const config_element_trackers;
 
 enum lttng_index_allocator_status lttng_index_allocator_alloc(
-               struct lttng_index_allocator*, uint64_t*)
+               struct lttng_index_allocator *a, uint64_t *b)
 {
        return LTTNG_INDEX_ALLOCATOR_STATUS_ERROR;
 }
 
-struct lttng_index_allocator* lttng_index_allocator_create(uint64_t)
+struct lttng_index_allocator* lttng_index_allocator_create(uint64_t a)
 {
        return NULL;
 }
 
-void lttng_index_allocator_destroy(struct lttng_index_allocator*)
+void lttng_index_allocator_destroy(struct lttng_index_allocator *a)
 {
 }
 
 uint64_t lttng_index_allocator_get_index_count(
-               struct lttng_index_allocator*)
+               struct lttng_index_allocator *a)
 {
        return -1ULL;
 }
 
 enum lttng_index_allocator_status lttng_index_allocator_release(
-               struct lttng_index_allocator*, uint64_t)
+               struct lttng_index_allocator *a, uint64_t b)
 {
        return LTTNG_INDEX_ALLOCATOR_STATUS_ERROR;
 }
This page took 0.033976 seconds and 4 git commands to generate.