From 283c6fa6cc573166ec4e41b70a9db57f8356f4aa Mon Sep 17 00:00:00 2001 From: Kienan Stewart Date: Thu, 13 Jul 2023 15:13:12 -0400 Subject: [PATCH] Fix: sessiond: crash when sending data_pending to an active session MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Jérémie Galarneau Change-Id: Iff46f87c7725d25c131a86ac3dbaed5c99b4d16b --- .gitignore | 1 + configure.ac | 1 + src/bin/lttng-sessiond/client.c | 6 +-- .../client/test_bug1480_assert_data_pending | 43 +++++++++++++++++++ tests/utils/testapp/Makefile.am | 1 + .../testapp/gen-data-pending/Makefile.am | 14 ++++++ tests/utils/testapp/gen-data-pending/main.cpp | 17 ++++++++ 7 files changed, 80 insertions(+), 3 deletions(-) create mode 100755 tests/regression/tools/client/test_bug1480_assert_data_pending create mode 100644 tests/utils/testapp/gen-data-pending/Makefile.am create mode 100644 tests/utils/testapp/gen-data-pending/main.cpp diff --git a/.gitignore b/.gitignore index ab6609952..5f3ba29b8 100644 --- a/.gitignore +++ b/.gitignore @@ -132,6 +132,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 a52bc7c90..8bf2edbc2 100644 --- a/configure.ac +++ b/configure.ac @@ -1210,6 +1210,7 @@ AC_CONFIG_FILES([ tests/utils/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.c b/src/bin/lttng-sessiond/client.c index c798234ae..fc26554c9 100644 --- a/src/bin/lttng-sessiond/client.c +++ b/src/bin/lttng-sessiond/client.c @@ -1977,12 +1977,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 index 000000000..cf7216e39 --- /dev/null +++ b/tests/regression/tools/client/test_bug1480_assert_data_pending @@ -0,0 +1,43 @@ +#!/bin/bash +# +# Copyright (C) 2023 Kienan Stewart +# +# 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 dcb83d2d9..3cdb551d5 100644 --- 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 index 000000000..70e0d01ea --- /dev/null +++ b/tests/utils/testapp/gen-data-pending/Makefile.am @@ -0,0 +1,14 @@ +# SPDX-FileCopyrightText: 2023 Kienan Stewart +# +# 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 index 000000000..e92ceb377 --- /dev/null +++ b/tests/utils/testapp/gen-data-pending/main.cpp @@ -0,0 +1,17 @@ +/* + * SPDX-FileCopyrightText: 2023 Kienan Stewart + * + * SPDX-License-Identifier: GPL-2.0-only + * + */ + +#include + +#include + +int main(int argc, const char **argv) +{ + assert(argc >= 2); + lttng_data_pending(argv[1]); + return 0; +} -- 2.34.1