Fix: sessiond: crash when sending data_pending to an active session

Observed Issue
==============

When a data_pending command is sent to an active session, the sessiond
crashes with the following assert

```
lttng-sessiond: client.cpp:2647: void* thread_manage_clients(void*): Assertion `cmd_ctx.reply_payload.buffer.size >= sizeof(*llm)' failed.
Error: 1 trace chunks are leaked by lttng-consumerd. This can be caused by an internal error of the session daemon.
```

Cause
=====

When a session is active, cmd.cpp:cmd_data_pending() returns
LTTNG_ERR_SESSION_STARTED. In client.cpp:process_client_msg(), this
return value causes the execution to go the the setup_error label. In
the setup_error label, no default LLM header is added to the reply,
meaning the reply has a zero size and triggering the assert above.

Solution
========

When cmd_data_pending() returns a value that is neither 0 nor 1, the
return code is set appropriately as follows:

  * when the return value is outside the range of lttng error codes,
  LTTNG_ERR_UNK is used
  * otherwise, the return value is used

The execution then jumps to the error label so that the default LLM
message header can be added.

Known Drawbacks
===============

None.

Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Iff46f87c7725d25c131a86ac3dbaed5c99b4d16b

diff --git a/.gitignore b/.gitignore
index 464920ea80df653dd661b8fac200f9b588381324..ca36c537b4ec884ff2bccce2edff19e4cdee09c1 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -134,6 +134,7 @@ compile_commands.json
 /tests/regression/ust/ust-dl/test_ust-dl
 /tests/regression/ust/multi-lib/exec-with-callsites
 /tests/regression/ust/multi-lib/exec-without-callsites
+/tests/utils/testapp/gen-data-pending/gen-data-pending
 /tests/utils/testapp/gen-syscall-events-callstack/gen-syscall-events-callstack
 /tests/utils/testapp/gen-ust-events/gen-ust-events
 /tests/utils/testapp/gen-ust-events-ns/gen-ust-events-ns

diff --git a/configure.ac b/configure.ac
index b085af884047557f49dc0000814978ccefe9e83b..b1a06ab19666d67c4ddce1fc7eca3f1052673848 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -1278,6 +1278,7 @@ AC_CONFIG_FILES([
        tests/utils/lttngtest/Makefile
        tests/utils/tap/Makefile
        tests/utils/testapp/Makefile
+       tests/utils/testapp/gen-data-pending/Makefile
        tests/utils/testapp/gen-ns-events/Makefile
        tests/utils/testapp/gen-kernel-test-events/Makefile
        tests/utils/testapp/gen-py-events/Makefile

diff --git a/src/bin/lttng-sessiond/client.cpp b/src/bin/lttng-sessiond/client.cpp
index 90ab4cdc02a38aef1aff28f4bfc1b7953f8d1421..a89c61e9c54d166a1eb106d202ffba5d9c699775 100644 (file)
--- a/src/bin/lttng-sessiond/client.cpp
+++ b/src/bin/lttng-sessiond/client.cpp
@@ -1948,12 +1948,12 @@ skip_domain:
                         * ret will be set to LTTNG_OK at the end of
                         * this function.
                         */
-               } else if (pending_ret < 0) {
+               } else if (pending_ret <= LTTNG_OK || pending_ret >= LTTNG_ERR_NR) {
                        ret = LTTNG_ERR_UNK;
-                       goto setup_error;
+                       goto error;
                } else {
                        ret = pending_ret;
-                       goto setup_error;
+                       goto error;
                }
 
                pending_ret_byte = (uint8_t) pending_ret;

diff --git a/tests/regression/tools/client/test_bug1480_assert_data_pending b/tests/regression/tools/client/test_bug1480_assert_data_pending
new file mode 100755 (executable)
index 0000000..cf7216e
--- /dev/null
+++ b/tests/regression/tools/client/test_bug1480_assert_data_pending
@@ -0,0 +1,43 @@
+#!/bin/bash
+#
+# Copyright (C) 2023 Kienan Stewart <kstewart@efficios.com>
+#
+# SPDX-License-Identifier: GPL-2.0-only
+#
+TEST_DESC="lttng-sessiond should not crash when receiving data-pending for an active session"
+CURDIR=$(dirname "$0")
+TESTDIR=$(realpath "${CURDIR}/../../../")
+# Test app for ust event
+TESTAPP_PATH="${TESTDIR}/utils/testapp"
+TESTAPP_NAME="gen-data-pending"
+TESTAPP_BIN="${TESTAPP_PATH}/${TESTAPP_NAME}/${TESTAPP_NAME}"
+SESSION_NAME=asdf
+
+NUM_TESTS=7
+
+# shellcheck source-path=SCRIPTDIR/../../../
+source "${TESTDIR}/utils/utils.sh"
+if [ ! -x "${TESTAPP_BIN}" ] ; then
+    BAIL_OUT "Test binary '${TESTAPP_BIN}' not detected or not executable"
+fi
+
+OUTPUT_DIR=$(mktemp -d)
+CHANNEL_NAME=asdf
+
+plan_tests "${NUM_TESTS}"
+print_test_banner "${TEST_DESC}"
+
+# shellcheck disable=SC2119
+start_lttng_sessiond
+
+create_lttng_session_ok "${SESSION_NAME}" "${OUTPUT_DIR}"
+enable_ust_lttng_channel_ok "${SESSION_NAME}" "${CHANNEL_NAME}"
+start_lttng_tracing_ok "${SESSION_NAME}"
+
+"${TESTAPP_BIN}" "${SESSION_NAME}"
+
+stop_lttng_tracing_ok "${SESSION_NAME}"
+destroy_lttng_session_ok "${SESSION_NAME}"
+# shellcheck disable=SC2119
+stop_lttng_sessiond
+rm -rf "${OUTPUT_DIR}"

diff --git a/tests/utils/testapp/Makefile.am b/tests/utils/testapp/Makefile.am
index 307059c0e0f7acddc4bc40043f87ec3372b07a44..b58c233d5fcca555652456b4c4b50b5bb93ccb8d 100644 (file)
--- a/tests/utils/testapp/Makefile.am
+++ b/tests/utils/testapp/Makefile.am
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 SUBDIRS = \
+         gen-data-pending \
          gen-kernel-test-events \
          gen-py-events \
          gen-syscall-events \

diff --git a/tests/utils/testapp/gen-data-pending/Makefile.am b/tests/utils/testapp/gen-data-pending/Makefile.am
new file mode 100644 (file)
index 0000000..70e0d01
--- /dev/null
+++ b/tests/utils/testapp/gen-data-pending/Makefile.am
@@ -0,0 +1,14 @@
+# SPDX-FileCopyrightText: 2023 Kienan Stewart <kstewart@efficios.com>
+#
+# SPDX-License-Identifier: GPL-2.0-only
+#
+
+AM_CPPFLAGS += -I$(top_srcdir)/tests/utils -I$(srcdir) \
+       -I$(top_srcdir)/tests/utils/testapp
+LIB_LTTNG_CTL = $(top_builddir)/src/lib/lttng-ctl/liblttng-ctl.la
+
+if HAVE_LIBLTTNG_UST_CTL
+noinst_PROGRAMS = gen-data-pending
+gen_data_pending_SOURCES = main.cpp
+gen_data_pending_LDADD = $(LIB_LTTNG_CTL)
+endif

diff --git a/tests/utils/testapp/gen-data-pending/main.cpp b/tests/utils/testapp/gen-data-pending/main.cpp
new file mode 100644 (file)
index 0000000..e92ceb3
--- /dev/null
+++ b/tests/utils/testapp/gen-data-pending/main.cpp
@@ -0,0 +1,17 @@
+/*
+ * SPDX-FileCopyrightText: 2023 Kienan Stewart <kstewart@efficios.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ *
+ */
+
+#include <lttng/lttng.h>
+
+#include <assert.h>
+
+int main(int argc, const char **argv)
+{
+       assert(argc >= 2);
+       lttng_data_pending(argv[1]);
+       return 0;
+}