lttng-tools.git
4 days ago.gitignore: ignore vscode files master
Jérémie Galarneau [Thu, 10 Oct 2019 19:32:08 +0000 (15:32 -0400)] 
.gitignore: ignore vscode files

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
5 days agoFix: lttng-elf.c: dereferencing pointer before null check
Francis Deslauriers [Wed, 9 Oct 2019 13:32:40 +0000 (09:32 -0400)] 
Fix: lttng-elf.c: dereferencing pointer before null check

Coverity report:
  CID 1405899 (#1 of 1): Dereference before null check (REVERSE_INULL)
  check_after_deref: Null-checking elf suggests that it may be null, but it has
  already been dereferenced on all paths leading to the check.

Reported-by: Coverity (1405899) Dereference before null check
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 days agoFix: sessiond: unbounded elf section data size allocation
Jérémie Galarneau [Tue, 8 Oct 2019 21:34:51 +0000 (17:34 -0400)] 
Fix: sessiond: unbounded elf section data size allocation

The size of ELF sections is read from a user-provided file descriptor
to an ELF file which could be malformed. In theory it would not
really be a problem as the run-as process is automatically restarted
after a crash (e.g. SIGBUS).

The alloctions are now bounded to the smallest of 512MB or the
file's size. The limit is kept high to accomodate very large
binaries and not impose an artificial limitation.

In time, this should be replaced by an mmap() of the section's
data rather than copying to a private set of pages.

1405558 Untrusted value as argument

The argument could be controlled by an attacker, who could invoke the
function with arbitrary values (for example, a very high or negative
buffer size).

In lttng_elf_get_sdt_probe_offsets: An unscrutinized value from an
untrusted source used as argument to a function (for example, a buffer
size) (CWE-20)

Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 days agoFix: sessiond: double socket close on allocation failure
Jérémie Galarneau [Tue, 8 Oct 2019 19:15:28 +0000 (15:15 -0400)] 
Fix: sessiond: double socket close on allocation failure

The application registration thread performs a double close() on
an application socket whenever it fails to allocate a ust_command.

Assign `-1` to `sock` after the initial close() to follow the
pattern of other close paths.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 days agoFix: sessiond: TOCTOU error on save of session configuration
Jérémie Galarneau [Tue, 8 Oct 2019 18:18:31 +0000 (14:18 -0400)] 
Fix: sessiond: TOCTOU error on save of session configuration

The session_save() function checks for the existance and access rights
on the target session configuration filename before opening it. This
results in a TOCTOU (Time of check, time of use) problem.

Defer the check and error reporting to the run_as_open() call.

1191754 Time of check time of use
An attacker could change the filename's file association or other
attributes between the check and use.  In save_session: A check occurs
on a file's attributes before the file is used in a privileged
operation, but things may have changed (CWE-367)

Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 days agoFix: tests: replace truncation-prone logging helper
Jérémie Galarneau [Tue, 8 Oct 2019 18:01:54 +0000 (14:01 -0400)] 
Fix: tests: replace truncation-prone logging helper

The printerr() error logging scheme in test_utils_expand_path
is prone to unexpected truncations which results in a lot of
warnings when building using GCC 9.2.

It is replaced by a variable-argument macro that uses fprintf()
directly.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 days agoDIST OOT: use build_dir version.i file
Jonathan Rajotte [Tue, 15 May 2018 21:27:52 +0000 (17:27 -0400)] 
DIST OOT: use build_dir version.i file

Requires the change of priority for include file in AM_CPPFLAGS and
that the version.h file include a "system header" version.i instead of a
local version.

Enable the passing of value from version.i to a OOT build done from a distribution
tarball. Enable a packager to touch files in custom_modifications and
generate a valid version.i file to be used in a OOT build from tarball.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 days agoIntroduce EXTRA_VERSION_PATCHES
Jonathan Rajotte [Mon, 14 May 2018 20:03:12 +0000 (16:03 -0400)] 
Introduce EXTRA_VERSION_PATCHES

This allow third-party (packagers) to provide more information about
what custom patches were applied to the tree.

To do so, one can create emtpy files in "version/extra_patches/",
the filenames will be used to generate the EXTRA_VERSION_PATCHES
define.

Add this information to the debug log of lttng-relayd and lttng-sessiond.

Also append it at the end of the "version" command of the lttng binary.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 days agoUse EXTRA_VERSION_NAME and EXTRA_VERSION_DESCRIPTION
Jonathan Rajotte [Fri, 4 May 2018 18:52:02 +0000 (14:52 -0400)] 
Use EXTRA_VERSION_NAME and EXTRA_VERSION_DESCRIPTION

Add detailed version information to the debug log of lttng-relayd and
lttng-sessiond.

Append the extra version information at the end of the "version" command
of the lttng binary.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 days agoIntroduce EXTRA_VERSION_NAME and EXTRA_VERSION_DESCRIPTION
Jonathan Rajotte [Fri, 4 May 2018 14:53:57 +0000 (10:53 -0400)] 
Introduce EXTRA_VERSION_NAME and EXTRA_VERSION_DESCRIPTION

On version.i generation, check the content of the following files :

  * "extra_version_name"

    The first line is used to populate the EXTRA_VERSION_NAME pre-processor
    define statement.

  * "extra_version_description"

    The content is used to populate the EXTRA_VERSION_DESCRIPTION pre-processor
    define statement.

    It should contain the description of local modifications done to the tree.
    This can be used by distribution packager to specify what changes were
    applied locally. Mostly in the form of patch/commit name. All non-alpha
    numeric characters are converted to "-".

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 days agoFix: relayd: Dereference after null check
Francis Deslauriers [Tue, 1 Oct 2019 15:43:39 +0000 (11:43 -0400)] 
Fix: relayd: Dereference after null check

There is no legitimate case where a stream's trace chunk would be NULL
while receiving a data packet. It could only result from an internal
error. Hence, stream->trace_chunk != NULL can be considered a
pre-condition of this function.

Coverity report:
  CID 1404937 (#1 of 1): Dereference after null check (FORWARD_NULL)
  11. var_deref_model: Passing null pointer stream->index_file to
  relay_index_set_file, which dereferences it

Reported-by: Coverity (1404937) Dereference after null check
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 days agoFix: sessiond: app sock and notif shm not created by the main thread
Jérémie Galarneau [Mon, 7 Oct 2019 23:34:08 +0000 (19:34 -0400)] 
Fix: sessiond: app sock and notif shm not created by the main thread

The application registration socket and application notification
shared memory are not created by the session daemon's main thread
which causes calls to umask() to be issued from multiple threads.

Since umask manipulates a process-wide ressource, it should be only be
used by a single thread.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
7 days agoFix: sessiond: client socket not created by the main thread
Jérémie Galarneau [Mon, 7 Oct 2019 22:26:51 +0000 (18:26 -0400)] 
Fix: sessiond: client socket not created by the main thread

The client socket is not created by the session daemon's main thread
which causes calls to umask() to be issued from multiple threads.

Since umask manipulates a process-wide ressource, it should be only be
used by a single thread.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
10 days agoFix: relayd: Dereference before null check
Francis Deslauriers [Fri, 4 Oct 2019 11:42:36 +0000 (07:42 -0400)] 
Fix: relayd: Dereference before null check

Coverity report:
  CID 1405858 (#1 of 1): Dereference before null check
  (REVERSE_INULL)check_after_deref: Null-checking base_path suggests that
  it may be null, but it has already been dereferenced on all paths
  leading to the check.

Reported-by: Coverity (1405858) Dereference before null check
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
10 days agoFix: relayd: unchecked return values
Francis Deslauriers [Tue, 1 Oct 2019 19:46:35 +0000 (15:46 -0400)] 
Fix: relayd: unchecked return values

Coverity reports:
  CID 1171571 (#1 of 1): Unchecked return value (CHECKED_RETURN)
  24. check_return: Calling compat_epoll_add without checking return
  value (as is done elsewhere 11 out of 13 times).

  CID 1171572 (#1 of 1): Unchecked return value (CHECKED_RETURN)
  24. check_return: Calling compat_epoll_add without checking return
  value (as is done elsewhere 11 out of 13 times).

Reported-by: Coverity (1171571) Unchecked return value
Reported-by: Coverity (1171572) Unchecked return value
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
10 days agoFix: ust-consumer.c: Double unlock of channel lock
Francis Deslauriers [Tue, 1 Oct 2019 15:06:30 +0000 (11:06 -0400)] 
Fix: ust-consumer.c: Double unlock of channel lock

Coverity report:
  CID 1404942 (#1 of 1): Double unlock (LOCK)
  15. double_unlock: pthread_mutex_unlock unlocks channel->lock while it is
  unlocked.

Reported-by: Coverity (1404942) Double unlock
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
10 days agoFix: sessiond: Dereference before null check
Francis Deslauriers [Tue, 1 Oct 2019 14:35:28 +0000 (10:35 -0400)] 
Fix: sessiond: Dereference before null check

Coverity report:
  CID 1404933 (#1 of 1): Dereference before null check
  (REVERSE_INULL)check_after_deref: Null-checking
  session->chunk_being_archived suggests that it may be null, but it has
  already been dereferenced on all paths leading to the check.

Reported-by: Coverity (1404933) Dereference before null check
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
10 days agoFix: sessiond: Dereference after null check
Francis Deslauriers [Tue, 1 Oct 2019 14:19:49 +0000 (10:19 -0400)] 
Fix: sessiond: Dereference after null check

Coverity report:
  CID 1404943 (#1 of 1): Dereference after null check (FORWARD_NULL)9.
  var_deref_model: Passing null pointer session->chunk_being_archived to
  lttng_trace_chunk_get_id, which dereferences it.

Reported-by: Coverity (1404943) Dereference after null check
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: relayd: Explicit null dereferenced
Francis Deslauriers [Fri, 27 Sep 2019 15:49:23 +0000 (11:49 -0400)] 
Fix: relayd: Explicit null dereferenced

Coverity warns about an explicit null dereference on an early exit path.

Coverity report:
  CID 1405577 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)7.
  var_deref_model: Passing null pointer previous_stream_fd to
  stream_fd_put, which dereferences it.

Reported-by: Coverity (1405577) Explicit null dereferenced
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoCleanup: relayd: Logically dead code
Francis Deslauriers [Fri, 27 Sep 2019 15:56:24 +0000 (11:56 -0400)] 
Cleanup: relayd: Logically dead code

Coverity report:
  CID 1404935 (#1 of 1): Logically dead code (DEADCODE)dead_error_line:
  Execution cannot reach the expression 0UL inside this statement:
  base_path_len = (base_path

Reported-by: Coverity (1404935) Logically dead code
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: enable_events.c: typo in `WARN()` message
Francis Deslauriers [Tue, 1 Oct 2019 13:04:12 +0000 (09:04 -0400)] 
Fix: enable_events.c: typo in `WARN()` message

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoCleanup: enable_events.c: fix erroneous comment
Francis Deslauriers [Tue, 1 Oct 2019 13:12:47 +0000 (09:12 -0400)] 
Cleanup: enable_events.c: fix erroneous comment

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoCleanup: relayd: identical code for different branches
Francis Deslauriers [Tue, 1 Oct 2019 13:35:19 +0000 (09:35 -0400)] 
Cleanup: relayd: identical code for different branches

Coverity report:
  CID 1404928 (#1 of 1): Identical code for different branches
  (IDENTICAL_BRANCHES)identical_branches: The same code is executed when
  the condition ret < 0 is true or false, because the code in the if-then
  branch and after the if statement is identical. Should the if statement
  be removed?

Reported-by: Coverity (1404928) Identical code for different branches
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: common: Unchecked return value of `closedir()`
Francis Deslauriers [Tue, 1 Oct 2019 13:43:03 +0000 (09:43 -0400)] 
Fix: common: Unchecked return value of `closedir()`

Coverity report:
  CID 1404930 (#1 of 1): Unchecked return value (CHECKED_RETURN)1.
  check_return: Calling closedir without checking return value (as is done
  elsewhere 5 out of 6 times).

Reported-by: Coverity (1404930) Unchecked return value
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: relayd: Dereference after null check
Francis Deslauriers [Tue, 1 Oct 2019 13:48:49 +0000 (09:48 -0400)] 
Fix: relayd: Dereference after null check

Coverity report:
  CID 1404934 (#1 of 1): Dereference after null check (FORWARD_NULL)
  11. var_deref_model: Passing null pointer element to
  trace_chunk_registry_ht_element_put, which dereferences it.

Reported-by: Coverity (1404934) Dereference after null check
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: Tests: test_session.c: Structurally dead code
Francis Deslauriers [Tue, 1 Oct 2019 15:50:43 +0000 (11:50 -0400)] 
Fix: Tests: test_session.c: Structurally dead code

Coverity report:
  CID 1400680 (#1 of 1): Structurally dead code (UNREACHABLE)
  unreachable: This code cannot be reached: ret = 0;.

Reported-by: Coverity (1400680) Structurally dead code
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: session-descriptor.c: Dereference before null check
Francis Deslauriers [Tue, 1 Oct 2019 18:21:00 +0000 (14:21 -0400)] 
Fix: session-descriptor.c: Dereference before null check

Coverity report:
  CID 1400682 (#1 of 1): Dereference before null check
  (REVERSE_INULL)check_after_deref: Null-checking descriptor suggests that
  it may be null, but it has already been dereferenced on all paths
  leading to the check.

Reported-by: (1400682) Dereference before null check
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: common: Dereference after null check
Francis Deslauriers [Tue, 1 Oct 2019 20:06:04 +0000 (16:06 -0400)] 
Fix: common: Dereference after null check

Coverity report:
  CID 1404929 (#1 of 1): Dereference after null check (FORWARD_NULL)
  25. var_deref_model: Passing null pointer directory_to_rename to
  lttng_directory_handle_rename_as_user, which dereferences it.

Reported-by: Coverity (1404929) Dereference after null check
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: test_utils_compat_poll.c: Unchecked return value
Francis Deslauriers [Tue, 1 Oct 2019 20:22:37 +0000 (16:22 -0400)] 
Fix: test_utils_compat_poll.c: Unchecked return value

Coverity report:
  CID 1401360 (#1 of 1): Unchecked return value (CHECKED_RETURN)4.
  check_return: Calling compat_epoll_create without checking return value
  (as is done elsewhere 6 out of 7 times).

Reported-by: Coverity (1401360) Unchecked return value
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: liblttng-ctl: wrong variable used during argument validation
Jérémie Galarneau [Thu, 3 Oct 2019 00:28:39 +0000 (20:28 -0400)] 
Fix: liblttng-ctl: wrong variable used during argument validation

Local 'handle' variable is used to check for NULL arguments while
the provided argument is named '_handle'. This results in failures
to destroy a session.

Rename the variable used in the argument check.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: liblttng-ctl: ABI-breaking size change of lttng_session struct
Jérémie Galarneau [Wed, 2 Oct 2019 22:04:43 +0000 (18:04 -0400)] 
Fix: liblttng-ctl: ABI-breaking size change of lttng_session struct

abidiff reports that the size of struct lttng_session has changed
since 2.10:

  [C]'function int lttng_list_sessions(lttng_session**)' at lttng-ctl.c:2065:1 has some indirect sub-type changes:
    parameter 1 of type 'lttng_session**' has sub-type changes:
      in pointed to type 'lttng_session*':
        in pointed to type 'struct lttng_session' at session.h:38:1:
          type size changed from 35008 to 35072 (in bits)
          1 data member deletion:
            'char lttng_session::padding[12]', at offset 34912 (in bits) at session.h:50:1

          1 data member insertion:
            'union {char padding[12]; void* ptr;} lttng_session::extended', at offset 34944 (in bits) at session.h:57:1

The offset after the 'live_timer_interval' field is aligned on 4
bytes, but not on 8 bytes. This causes some compilers (such as gcc and
clang) to align the following 'extended' union on 8 bytes, making the
overall structure larger.

To preserve the size of 'struct lttng_session', four bytes of padding
are added after 'live_timer_interval', resulting in an aligned offset
for both bitnesses.

The 'extended' union's padding is reduced from 12 to 8 bytes,
essentially ensuring that 'ptr' always occupies 8 bytes, even on
32-bit builds.

Tested on clang and gcc for x64, x86, PPC32, PPC64, ARM, ARM64,
AVR, MIPS, MIPS64, and MSVC (32-bit and 64-bit).

Reviewed-by: Michael Jeanson <michael.jeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: liblttng-ctl: config and mi strings inadvertantly exported
Jérémie Galarneau [Wed, 2 Oct 2019 21:55:06 +0000 (17:55 -0400)] 
Fix: liblttng-ctl: config and mi strings inadvertantly exported

abidiff reports that a number of configuration and MI string symbols
are exported by liblttng-ctl:

'const char* const config_event_type_userspace_probe'    {config_event_type_userspace_probe}
'const char* const mi_lttng_element_command_disable_rotation'    {mi_lttng_element_command_disable_rotation}
'const char* const mi_lttng_element_command_enable_rotation'    {mi_lttng_element_command_enable_rotation}
'const char* const mi_lttng_element_command_rotate'    {mi_lttng_element_command_rotate}

Those strings are marked as LTTNG_HIDDEN.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: liblttng-ctl: compat_sync_file_range inadvertantly exported
Jérémie Galarneau [Wed, 2 Oct 2019 21:53:58 +0000 (17:53 -0400)] 
Fix: liblttng-ctl: compat_sync_file_range inadvertantly exported

abidiff reports that compat_sync_file_range is exported by
liblttng-ctl.

'function int compat_sync_file_range(int, off64_t, off64_t, unsigned int)'    {compat_sync_file_range}

Mark the function as LTTNG_HIDDEN.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: liblttng-ctl: poll compatibility symbols inadvertently exported
Jérémie Galarneau [Wed, 2 Oct 2019 21:25:00 +0000 (17:25 -0400)] 
Fix: liblttng-ctl: poll compatibility symbols inadvertently exported

abidiff reports that the following poll/epoll compatibility layer
symbols were inadventently exported by liblttng-ctl:

'function int compat_epoll_add(compat_epoll_event*, int, uint32_t)'    {compat_epoll_add}
'function int compat_epoll_create(compat_epoll_event*, int, int)'    {compat_epoll_create}
'function int compat_epoll_del(compat_epoll_event*, int)'    {compat_epoll_del}
'function int compat_epoll_mod(compat_epoll_event*, int, uint32_t)'    {compat_epoll_mod}
'function int compat_epoll_set_max_size()'    {compat_epoll_set_max_size}
'function int compat_epoll_wait(compat_epoll_event*, int, bool)'    {compat_epoll_wait}

Those functions and their 'poll' equivalent are marked as
LTTNG_HIDDEN.

The poll_max_size variable is also made static and removed from the
compatibility header since it is never used apart from within the
implementation files.

'unsigned int poll_max_size'    {poll_max_size}

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoFix: lttng-ctl: unvalidated session destruction handle API arguments
Jérémie Galarneau [Wed, 2 Oct 2019 18:46:26 +0000 (14:46 -0400)] 
Fix: lttng-ctl: unvalidated session destruction handle API arguments

The liblttng-ctl API is not performance sensitive and normally adopts
a defensive stance with regards to supplied arguments. The session
destruction handle API introduced in 2.11 does not check user-supplied
arguments for NULLs which does not fit with existing liblttng-ctl API
conventions.

Add NULL checks for all arguments which cannot be legitimately left
NULL and return a suitable "invalid parameters" return code.

Moreover, note that lttng_destroy_session_ext() is now used by
lttng_destroy_session(), which previously checked for a NULL session
name. Not checking for this case in the new 'ext' version introduced a
change in behaviour.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
12 days agoDocs: document the session destruction handle API
Jérémie Galarneau [Wed, 2 Oct 2019 18:42:26 +0000 (14:42 -0400)] 
Docs: document the session destruction handle API

Add function headers to the session destruction API declared in
destruction-handle.h.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
13 days agoFix: Tests: Segfault in `test_utils_expand_path()`
Francis Deslauriers [Fri, 27 Sep 2019 19:14:17 +0000 (15:14 -0400)] 
Fix: Tests: Segfault in `test_utils_expand_path()`

Background
==========
I have a file named "/a" on my file system (don't ask why).

Issue
=====
While running the `test_utils_expand_path` test case on my machine, I
get a Segfault. Here is the gdb backtrace:

  #0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:62
  #1  0x0000555555559eb7 in expand_double_slashes_dot_and_dotdot (path=0x0) at utils.c:223
  #2  0x000055555555a2d8 in _utils_expand_path (path=0x55555556b250 "/a/b/c/d/e", keep_symlink=true) at utils.c:384
  #3  0x000055555555a408 in utils_expand_path (path=0x55555556b250 "/a/b/c/d/e") at utils.c:423
  #4  0x000055555555859e in test_utils_expand_path () at test_utils_expand_path.c:291
  #5  0x00005555555589b0 in main (argc=1, argv=0x7fffffffe5e8) at test_utils_expand_path.c:352

I get this backtrace because the function `utils_partial_realpath()`
returns NULL when it tries to expand the "/a/b/c/d/e" path and realize
that it could not exist since "/a" is a file and not a directory.

Anyways, the returned NULL pointer is ignored and directly used in the
`expand_double_slashes_dot_and_dotdot()` function right after.

This configuration ("/a" being a file) is expected to fail but not to segfault.
It could be reproduce in a real scenario when creating directory
structures.

Solution
========
Return an error if `utils_partial_realpath()` returns NULL.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
13 days agoFix: lttng-ctl: missing __cplusplus closing brace
Jérémie Galarneau [Tue, 1 Oct 2019 17:54:22 +0000 (13:54 -0400)] 
Fix: lttng-ctl: missing __cplusplus closing brace

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 weeks agoFix: trace chunk reported unknown before close command execution
Jérémie Galarneau [Mon, 30 Sep 2019 19:57:33 +0000 (15:57 -0400)] 
Fix: trace chunk reported unknown before close command execution

The check for the existence of a trace chunk is done by using the
regular 'find' operations on a trace chunk registry in both the relay
and consumer daemons.

The 'find' operations attempt to acquire a reference to the trace
chunk being looked-up. On failure to acquire a reference, a trace
chunk is reported as being "unknown". The rotation completion check
uses this mechanism to determine if a trace chunk is still in use.

A close command defers a given operation until a trace chunk is no
longer in use (when its last reference is dropped). Hence, a thread
can attempt to 'find' a trace chunk, fail to acquire a reference, and
fail to see the effects of the close command.

In other words, the thread that has dropped the last reference to
the chunk could still be running the close command of a trace chunk
that is reported to be "unknown" by the consumer and relay daemons.

To fix this, dedicated "chunk_exists" operations are introduced. These
operations do not attempt to acquire a trace chunk. They only look it
up in the registry's internal hash table. As the removal of the trace
chunk from the hash table is performed _after_ the execution of its
close command, it provides a reliable way to ensure that a chunk that
had a close command could complete it before being reported as
"unknown"/no longer in use.

In terms of provided guarantees, this changes the moment at which a
trace chunk is considered to no longer exist from the moment its last
reference was dropped to the moment after the execution of its close
command has completed.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 weeks agoFix: sessiond: leak of application socket on chmod failure
Jérémie Galarneau [Wed, 25 Sep 2019 22:04:14 +0000 (18:04 -0400)] 
Fix: sessiond: leak of application socket on chmod failure

The apps_sock fd is leaked whenever the chmod of the application
socket fails. Add a clean-up error path.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 weeks agosessiond: clean-up: silence warning that agent event is leaked
Jérémie Galarneau [Wed, 25 Sep 2019 21:06:23 +0000 (17:06 -0400)] 
sessiond: clean-up: silence warning that agent event is leaked

Both Coverity and scan-build got confused by this
function. Essentially, they warn that aevent can be leaked if
it is created in an already enabled state. We know that this can't
happen as the events are created in a disabled state.

Add an assert that created events are not enabled to help the static
analyzers. This could also catch the leak should this change in the
future.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 weeks agoFix: tests: leak of prefix on error to register lttng namespace
Jérémie Galarneau [Wed, 25 Sep 2019 20:55:35 +0000 (16:55 -0400)] 
Fix: tests: leak of prefix on error to register lttng namespace

prefix should always be free'd in this function.

1402045 Resource leak
The system resource will not be reclaimed and reused, reducing the
future availability of the resource.

In register_lttng_namespace: Leak of memory or pointers to system
resources (CWE-404)

Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 weeks agoFix: use newly created event filter for condition check
Jonathan Rajotte [Tue, 24 Sep 2019 15:24:17 +0000 (11:24 -0400)] 
Fix: use newly created event filter for condition check

The following commit introduced a regression while
fixing the filter and filter_expression ownership.

commit b0a23296344e57bd2e48e62ec2d7e0d8a38661bb
Author: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Date:   Sat Jan 12 14:53:56 2019 -0500

    Fix: leak of filter bytecode and expression on agent event re-enable

    The agent subsystem does not properly assume the clean-up of an
    event's filter bytecode and expression when a previously disabled
    event is re-enabled.

    This change ensures that the ownership of both the filter bytecode
    and expression is assumed by the agent subsystem and discarded
    when a matching event is found.

    Steps to reproduce the leak:
    $ lttng create
    $ lttng enable-event --python allo --filter 'a[42] == 241'
    $ lttng disable-event --python allo
    $ lttng enable-event --python allo --filter 'a[42] == 241'

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Setting the "filter" object to NULL prevents the call to
add_filter_app_ctx when needed.

We use the filter from the newly created event to
perform the check and the call to add_filter_app_ctx.

Fixes coverity #1399733

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 weeks agoFix: lttng-crash: detect truncated files
Mathieu Desnoyers [Mon, 23 Sep 2019 18:31:33 +0000 (14:31 -0400)] 
Fix: lttng-crash: detect truncated files

Detect truncated files which size is smaller than the ring buffer
header.

This can be caused by a situation where sessiond is killed with SIGKILL
while doing a metadata regenerate command.

Without this fix, lttng-crash is killed with a "Bus error" when
encountering a truncated file.

Fixes: #1166
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 weeks agoFix: sessiond: fs.protected_regular sysctl breaks app registration
Jérémie Galarneau [Tue, 24 Sep 2019 05:10:58 +0000 (01:10 -0400)] 
Fix: sessiond: fs.protected_regular sysctl breaks app registration

I observed that userspace tracing no longer worked when an
instrumented application (linked against liblttng-ust) was launched
before the session daemon.

While investigating this, I noticed that the shm_open() of
'/lttng-ust-wait-8' failed with EACCES. As the permissions on the
'/dev/shm' directory and the file itself should have allowed the
session daemon to open the shm, this pointed to a change in kernel
behaviour.

Moreover, it appeared that this could only be reproduced on my
system (running Arch Linux) and not on other systems.

It turns out that Linux 4.19 introduces a new protected_regular sysctl
to allow the mitigation of a class of TOCTOU security issues related
to the creation of files and FIFOs in sticky directories.

When this sysctl is not set to '0', it specifically blocks the way the
session daemon attempts to open the app notification shm that an
application has already created.

To quote a comment added in linux's fs/namei.c as part of 30aba6656f:

```
Block an O_CREAT open of a FIFO (or a regular file) when:
  - sysctl_protected_fifos (or sysctl_protected_regular) is enabled
  - the file already exists
  - we are in a sticky directory
  - we don't own the file
  - the owner of the directory doesn't own the file
  - the directory is world writable
```

While the concerns that led to the inclusion of this patch are valid,
the risks that are being mitigated do not apply to the session
daemon's and instrumented application's use of this shm. This shm is
only used to wake-up applications and get them to attempt to connect
to the session daemon's application socket. The application socket is
the part that is security sensitive. At worst, an attacker controlling
this shm could wake up the UST thread in applications which would then
attempt to connect to the session daemon.

Unfortunately (for us, at least), systemd v241+ sets the
protected_regular sysctl to 1 by default (see systemd commit
27325875), causing the open of the shm by the session daemon to fail.

Introduce a fall-back to attempt a shm_open without the O_CREAT flag
when opening it with 'O_RDWR | O_CREAT' fails. The comments detail the
reason why those attempts are made in that specific order.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agorelayd: clean-up: mix-up between LTTNG_PATH_MAX and LTTNG_NAME_MAX
Jérémie Galarneau [Mon, 23 Sep 2019 18:46:42 +0000 (14:46 -0400)] 
relayd: clean-up: mix-up between LTTNG_PATH_MAX and LTTNG_NAME_MAX

LTTNG_PATH_MAX and LTTNG_NAME_MAX are mixed up in
cmd_create_session_2_4(). While Coverity warns of a possible buffer
overrun, this is not possible since the length of the received
buffer is correctly checked against LTTNG_NAME_MAX.

Change the use of LTTNG_PATH_MAX for LTTNG_NAME_MAX even though
strcpy() could be used safely here.

1405634 Out-of-bounds access
Access of memory not owned by this buffer may cause crashes or incorrect computations.
In relay_create_session: Out-of-bounds access to a buffer (CWE-119)

Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agoFix: destroy command: put consumer output after destroy notifier
Mathieu Desnoyers [Fri, 20 Sep 2019 21:41:14 +0000 (17:41 -0400)] 
Fix: destroy command: put consumer output after destroy notifier

The destroy notifier needs to access the consumer output to format
the absolute path to the last chunk.

The observed problematic behavior can be observed by doing a
rotate and a destroy command in quick succession. Sometimes,
the resulting path printed by the destroy command is incomplete:

e.g. /archives/20190920T163616-0400-20190920T163618-0400-1

when we would expect:

/home/efficios/lttng-traces/auto-20190920-164425/archives/20190920T164437-0400-20190920T164439-0400-1

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agoRefactor: remove logically dead code
Jonathan Rajotte [Fri, 20 Sep 2019 15:40:37 +0000 (11:40 -0400)] 
Refactor: remove logically dead code

Fixes coverity #1400681

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agoFix: Null check before destroying health_sessiond
Jonathan Rajotte [Fri, 20 Sep 2019 15:15:12 +0000 (11:15 -0400)] 
Fix: Null check before destroying health_sessiond

Fixes coverity #1399735

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agoFix: Move initialization of queue_pipe_fd after null check of handle
Jonathan Rajotte [Fri, 20 Sep 2019 15:07:55 +0000 (11:07 -0400)] 
Fix: Move initialization of queue_pipe_fd after null check of handle

Fixes coverity #1399732

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agoFix: release reference to new chunk on copy error
Jonathan Rajotte [Fri, 20 Sep 2019 14:47:30 +0000 (10:47 -0400)] 
Fix: release reference to new chunk on copy error

Fixes coverity #1405775

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agoFix: Close socket handle on error
Jonathan Rajotte [Fri, 20 Sep 2019 15:34:21 +0000 (11:34 -0400)] 
Fix: Close socket handle on error

Fixes coverity #1399739

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agoFix: lttng: out-of-bound copy of arguments in 'view' command handler
Jérémie Galarneau [Thu, 19 Sep 2019 18:24:34 +0000 (14:24 -0400)] 
Fix: lttng: out-of-bound copy of arguments in 'view' command handler

The 'size' operand of memcpy() does not indicate the length of the
opts array; it is the size of the resulting array once the opts array
is concatenated with the options being added in this function. This
results in out-of-bound read(s) in the opts array.

Use 'sizeof(char *) * opts_len' as the length to copy at the beginning
of the resulting array.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agoRevert "lttng: fix: out-of-bounds copy of original 'view' command arguments"
Jérémie Galarneau [Thu, 19 Sep 2019 18:18:50 +0000 (14:18 -0400)] 
Revert "lttng: fix: out-of-bounds copy of original 'view' command arguments"

This reverts commit fc1465822a1075d98160bb74ae8e974ac8c010a8 as it
broke the 'lttng view' command on live sessions. The original commit
was meant to fix a warning, but it was (evidently) not an appropriate
change.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agoFix: relayd: session destruction does not complete in live mode
Jérémie Galarneau [Thu, 19 Sep 2019 17:48:20 +0000 (13:48 -0400)] 
Fix: relayd: session destruction does not complete in live mode

Live clients currently hold a reference to their relay_session's trace
chunk through the viewer streams. This causes the destruction of live
session to hang for as long as a client is consuming a session's
contents.

The reason for the hang itself is that the "quiet" rotation performed
as part of the session's destruction can never complete. Indeed, a
rotation completes when the last reference to the trace chunk being
archived is released. Since viewer streams hold references to this
trace chunk, they bound its lifetime to their own lifetime, resulting
in the destruction operation appearing to hang to users of
liblttng-ctl / the lttng client.

Since session rotations are not allowed for live sessions, it is safe
to presume that a session's output folder hierarchy will not change
over its lifetime with the exception of sub-folder additions, such as
in per-PID tracing mode where new traces may be added at any time
within the session's output directory.

The viewer session now copies its corresponding relay_session's trace
chunk to obtain a 'user' mode trace chunk. A user mode trace chunk
will prevent the viewer session from modifying the session output
hierarchy and prevent it from keeping the relay_session's trace chunk
"alive". The viewer stream simply use this trace chunk to perform
their filesystem operations, but they no longer extend the lifetime of
the relayd_session trace chunks.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agoAdd a copy method to the trace chunk interface
Jérémie Galarneau [Wed, 18 Sep 2019 16:34:07 +0000 (12:34 -0400)] 
Add a copy method to the trace chunk interface

Trace chunks may now be copied. This will allow, in a follow-up fix,
streams to use the trace chunk facilities without extending the
life-time of an existing chunk.

However, note that the copy of an "owner" chunk returns a "user"
chunk. While this may be surprising at first, it makes no sense for
two "owner" trace chunks to point to the same location.

In such a case, it would be legal for both trace chunks to arbitrarily
modify their underlying sub-folder hierarchies without coordinating
among themselves. This would break the existing "move to completed"
close command, for instance.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agorelayd: move viewer stream chunk reference release to destroy
Jérémie Galarneau [Thu, 19 Sep 2019 17:47:08 +0000 (13:47 -0400)] 
relayd: move viewer stream chunk reference release to destroy

Move the viewer stream's release of its trace chunk to the destroy
method (as opposed to the release method) as destroy is used directly
when an error occurs in the viewer stream's creation function.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agorelayd: move relay_session locking outside of make_viewer_streams
Jérémie Galarneau [Thu, 19 Sep 2019 17:22:17 +0000 (13:22 -0400)] 
relayd: move relay_session locking outside of make_viewer_streams

The locking period of the relay_session is slightly extended outside
of make_viewer_streams(). This makes a follow-up change easier to
review, but is also a clean-up cleaner as some of the session's
attribute are accessed without holding the lock (`live_timer` and
`id`) assuming (correctly, for the moment) that they don't change over
the lifetime of the relayd_session. It also allows us to move the
relayd_session locking logic from viewer_session_attach().

The signature of send_viewer_streams() is changed to accept the
session's id rather than a pointer to the relay_session, therefore
making the locking scope simpler to follow. Changing this makes
follow-up changes easier to review.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agoFix: release reference to trace chunk on index file creation failure
Jérémie Galarneau [Wed, 18 Sep 2019 17:47:26 +0000 (13:47 -0400)] 
Fix: release reference to trace chunk on index file creation failure

The reference to a trace chunk that is acquired in
_lttng_index_file_create_from_trace_chunk() is leaked on error.
Release the reference in the function's error path.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agotrace-chunk: clean-up: mark close command properties as static const
Jérémie Galarneau [Wed, 18 Sep 2019 16:33:15 +0000 (12:33 -0400)] 
trace-chunk: clean-up: mark close command properties as static const

Command properties should not be experted outside of trace-chunk.c
and should be const. Mark them as such.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 weeks agotrace-chunk: clean-up: misleading label name
Jérémie Galarneau [Wed, 18 Sep 2019 16:30:22 +0000 (12:30 -0400)] 
trace-chunk: clean-up: misleading label name

The 'end_unlock' label in lttng_trace_chunk_set_close_command() no
longer needs to unlock the chunk (and doesn't do it). Rename it to
'end' to alleviate any confusion in the future.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoust-consumer: fix: metadata stream lock taken before destroy
Jérémie Galarneau [Sun, 15 Sep 2019 19:10:20 +0000 (15:10 -0400)] 
ust-consumer: fix: metadata stream lock taken before destroy

The lock of a metadata stream is taken when calling the stream's
destroy function after the completion of a snapshot. This is invalid
and does not appear to protect anything.

I am guessin that this was meant to be an 'unlock' invoked in the
error path of this functions.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agosessiond: fix: memory leak of section name in elf parser
Jérémie Galarneau [Sun, 15 Sep 2019 19:01:33 +0000 (15:01 -0400)] 
sessiond: fix: memory leak of section name in elf parser

lttng_elf_get_section_name() returns a dynamically-allocated section
name. However, lttng_elf_get_section_hdr_by_name() never frees
this returned section name.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agokconsumer: clean-up: initialize ctf_index before populating it
Jérémie Galarneau [Sun, 15 Sep 2019 18:56:23 +0000 (14:56 -0400)] 
kconsumer: clean-up: initialize ctf_index before populating it

The 'offset' field of the ctf_index is used uninitialized when it is
populated. Its uninitialized value is read and stored back.  This is
not a problem as the offset is populated later-on.

Coveity reports
1405630 Uninitialized scalar variable
The variable will contain an arbitrary value left from earlier computations.
In lttng_kconsumer_read_subbuffer: Use of an uninitialized variable (CWE-457)

Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agosessiond: fix: strncpy called with source length
Jérémie Galarneau [Sun, 15 Sep 2019 18:27:40 +0000 (14:27 -0400)] 
sessiond: fix: strncpy called with source length

strncpy is called with the source's length in two cases in the
session save code. Use the destination and remaining destination
length as intended by the API.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agosessiond: fix: possible unaligned access in packed structure
Jérémie Galarneau [Sun, 15 Sep 2019 17:19:22 +0000 (13:19 -0400)] 
sessiond: fix: possible unaligned access in packed structure

A number of (potentially) unaligned pointers are used in client
command handlers to pass command parameters to other internal
functions. Fixing those forces a number of const-correctness
changes where contexts are passed as arguments.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agorelayd: clean-up: strncpy uses the length of the source as length
Jérémie Galarneau [Sat, 14 Sep 2019 20:53:22 +0000 (16:53 -0400)] 
relayd: clean-up: strncpy uses the length of the source as length

strncpy is used with the source length of both session_name and
hostname. This doesn't make sense at face value. However, this is not
a "bug" as both src and dst are guaranteed to be the same length in
all existing code paths.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agolttng: fix: out-of-bounds copy of original 'view' command arguments
Jérémie Galarneau [Sat, 14 Sep 2019 20:49:07 +0000 (16:49 -0400)] 
lttng: fix: out-of-bounds copy of original 'view' command arguments

A mix-up between 'size' and 'opts_len' can cause the copy of the
original arguments passed to the 'view' command can result in
out-of-bounds accesses.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agolttng: clean-up: silence bogus string truncation warning
Jérémie Galarneau [Sat, 14 Sep 2019 20:43:38 +0000 (16:43 -0400)] 
lttng: clean-up: silence bogus string truncation warning

gcc 9.1.0 reports
enable_events.c:827:2: warning: ‘strncpy’ output truncated copying 9 bytes from a string of length 11 [-Wstringop-truncation]
  827 |  strncpy(ret, preamble, length);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

gcc seems confused and using "sizeof" the preamble seems to make
it understand the code flow and not generate the warning anymore.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agosessiond: clean-up: mixed log levels enums used to look-up event
Jérémie Galarneau [Sat, 14 Sep 2019 20:27:45 +0000 (16:27 -0400)] 
sessiond: clean-up: mixed log levels enums used to look-up event

lttng_ust_loglevel_type and lttng_loglevel_type enums are mixed.  This
is correct as they share all values. A cast is added to silence a
clang warning. Normally we would assign the values of one to the
other. This is not practical here as the ust variant is defined by the
lttng-ust API and the other is part of the public lttng-ctl API.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agosessiond: fix: possible unaligned access in packed structure
Jérémie Galarneau [Sat, 14 Sep 2019 20:16:56 +0000 (16:16 -0400)] 
sessiond: fix: possible unaligned access in packed structure

Use a temporary snapshot_id in cmd_rotate_get_info() to obtain the
return value of cmd_snapshot_add_output() and explicitly assign it to
the reply communication structure. Otherwise, &reply.id may be
unaligned.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agosessiond: fix: possible unaligned access in packed structure
Jérémie Galarneau [Sat, 14 Sep 2019 20:09:35 +0000 (16:09 -0400)] 
sessiond: fix: possible unaligned access in packed structure

Use temporary control and data port variables to get rotation
destination ports and explicitly copy them to the return communication
structure.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agosessiond: fix: possible unaligned access in packed structure
Jérémie Galarneau [Sat, 14 Sep 2019 20:02:33 +0000 (16:02 -0400)] 
sessiond: fix: possible unaligned access in packed structure

'&rsock->sock.fd' is passed to consumer_send_fds and may result in an
unaligned pointer value. Use the ALIGNED_CONST_PTR macro to create
an aligned copy of the fd that is being passed.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agosessiond: fix: possible unaligned access in packed structure
Jérémie Galarneau [Sat, 14 Sep 2019 19:58:17 +0000 (15:58 -0400)] 
sessiond: fix: possible unaligned access in packed structure

Use a temporary relayd_session_id variable to prevent an unaligned
access in the packed communication structure 'msg'.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agolttng-ctl: fix: possible unaligned access in packed structure
Jérémie Galarneau [Sat, 14 Sep 2019 19:51:32 +0000 (15:51 -0400)] 
lttng-ctl: fix: possible unaligned access in packed structure

Wrap all lttng_domain copies with COPY_DOMAIN_PACKED which copies the
source domain to a temporary destination (on stack) and then assign
this temporary domain to the destination domain. This ensures the
compiler generates the code needed to perform the unaligned accesses
to the domain.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agolttng-ctl rotate: fix: possible unaligned access in packed structure
Jérémie Galarneau [Sat, 14 Sep 2019 18:38:44 +0000 (14:38 -0400)] 
lttng-ctl rotate: fix: possible unaligned access in packed structure

Fix the warnings that unaligned pointers can be passed as parameters
emitted when sampling rotation schedules.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agorunas: fix: possible unaligned access in packed structure
Jérémie Galarneau [Sat, 14 Sep 2019 18:31:40 +0000 (14:31 -0400)] 
runas: fix: possible unaligned access in packed structure

Fix the warning that an unaligned pointers can be passed as parameter
emitted in _extract_elf_symbol_offset.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoconsumer: fix: possible unaligned access in packed structure
Jérémie Galarneau [Sat, 14 Sep 2019 18:25:25 +0000 (14:25 -0400)] 
consumer: fix: possible unaligned access in packed structure

Fix the warnings that unaligned pointers can be passed as parameters
emitted when sampling buffer statistics.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoinet: fix: possible unaligned access in packed structure (inet/inet6)
Jérémie Galarneau [Thu, 12 Sep 2019 15:51:52 +0000 (11:51 -0400)] 
inet: fix: possible unaligned access in packed structure (inet/inet6)

Fix the warnings that unaligned pointers can be passed as parameters
emitted when building inet.c and inet6.c.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoconsumer: fix: unaligned accesses to index fields
Jérémie Galarneau [Fri, 13 Sep 2019 20:48:18 +0000 (16:48 -0400)] 
consumer: fix: unaligned accesses to index fields

The ctf_index structure, being part of the ABI, is explicitly packed
using the LTTNG_PACKED macro. However, populating it by using pointers
to its members is not acceptable as it may cause the ust and kernel
tracer APIs to populate write their return values using unaligned
pointers.

Use automatic storage variables to fetch the various index fields and
populate the index at-once using a compound literal.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agolttng-sessiond: clean-up: set free'd pointer to NULL
Jérémie Galarneau [Thu, 12 Sep 2019 14:23:23 +0000 (10:23 -0400)] 
lttng-sessiond: clean-up: set free'd pointer to NULL

Set the wait_node and ust_cmd pointers to NULL after they have been
free'd to make this function easier to follow. This may also help
scan-build analyze this function as it gets confused about the values
of those variables. Currently, scan-build (clang 8.0.1) reports a
use-after free of both variables when 'app' is simultaneously null and
non-null... you read that right.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agolttng: fix: potential 0-length allocation in pid list parsing
Jérémie Galarneau [Thu, 12 Sep 2019 14:17:41 +0000 (10:17 -0400)] 
lttng: fix: potential 0-length allocation in pid list parsing

Check that count is > 0 before allocating pid list. This would
only happen after a prior error, but check it anyway.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoTests: fix: uninitialized session_id used on list_sessions failure
Jérémie Galarneau [Thu, 12 Sep 2019 14:12:41 +0000 (10:12 -0400)] 
Tests: fix: uninitialized session_id used on list_sessions failure

Stop live test when list_sessions() fails since the session_id used
further on would be uninitialized.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoTests: fix: uninitialized values passed to close() on error
Jérémie Galarneau [Thu, 12 Sep 2019 14:08:44 +0000 (10:08 -0400)] 
Tests: fix: uninitialized values passed to close() on error

The fds array is not initialized resulting in uninitialized file
descriptors being passed to close() when an error is encountered in
the epoll-setting loop.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoClean-up: assert that get_count_order() returns a positive value
Jérémie Galarneau [Thu, 12 Sep 2019 14:02:06 +0000 (10:02 -0400)] 
Clean-up: assert that get_count_order() returns a positive value

The callers of get_count_order() don't handle negative values
correctly. However, those should not happen without a previous
bug. Assert that its return value is >= 0.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoClean-up: suppress bogus scan-build warning
Jérémie Galarneau [Thu, 12 Sep 2019 13:50:29 +0000 (09:50 -0400)] 
Clean-up: suppress bogus scan-build warning

scan-build seems confused about the address passed to free() and
reports that chunk elements are passed with an offset of 8 bytes from
the address that was returned by calloc().

Move the inner-trace chunk of an element to the start of the element
wrapper-structure to silence this warning.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoTests: fix: leak caused by misuse of realloc in multi-lib-test
Jérémie Galarneau [Wed, 11 Sep 2019 17:30:18 +0000 (13:30 -0400)] 
Tests: fix: leak caused by misuse of realloc in multi-lib-test

realloc() can return NULL, in which case 'libraries' will never
be free'd in the case where the reallocation happens outside of
the while()'s first iteration.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agosessiond: clean-up: init ret value of _session_set_trace_chunk_no_lock_check
Jérémie Galarneau [Wed, 11 Sep 2019 17:22:32 +0000 (13:22 -0400)] 
sessiond: clean-up: init ret value of _session_set_trace_chunk_no_lock_check

This fixes are warning that 'ret' may not be initialized when a
session has no ust nor kernel session. In such a case, the sessiond
would never start a session and thus would not assign a trace chunk to
the session. Still, this can be confuging so it is better-off being
fixed.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agosessiond: fix: print_escaped_ctf_string mishandles empty string
Jérémie Galarneau [Wed, 11 Sep 2019 17:16:48 +0000 (13:16 -0400)] 
sessiond: fix: print_escaped_ctf_string mishandles empty string

The return value of print_escaped_ctf_string() is uninitialized in the
case of an empty string ("").

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agolttng-ctl: fix: lttng_data_pending confuses communication status
Jérémie Galarneau [Wed, 11 Sep 2019 16:18:15 +0000 (12:18 -0400)] 
lttng-ctl: fix: lttng_data_pending confuses communication status

lttng_ctl_ask_sessiond can return a positive value even though it
failed to receive the variable length payload of a session message
reply. In this case, lttng_ctl_ask_sessiond ends up calling into
lttng_ctl_ask_sessiond_fds_varlen() which will return the (negated)
error code returned by the session daemon if it was not LTTNG_OK.

The peer could return anything here, which lttng_data_pending will end
up interpreting as the length of the variable data that was received.

In this case, if the sessiond returns '-1', '1' will be returned to
lttng_data_pending, which it will interpret as being the length of the
'data_pending' byte flag. It will then dereference 'pending', which is
NULL, and (most likely) crash.

Check for NULL on top of checking for the return code. This
communication layer needs love as much as it needs a bulldozer.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agorelayd: fix: rotate_truncate_stream() assumes non-null next chunk
Jérémie Galarneau [Wed, 11 Sep 2019 16:03:38 +0000 (12:03 -0400)] 
relayd: fix: rotate_truncate_stream() assumes non-null next chunk

While the protocol doesn't allow a stream rotation position in the
past when rotating to a "NULL" trace chunk, a misbehaving peer could
express this. Report the protocol error and abort the truncation
operation in this case.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoFix: dereference of NULL pointer in stream_write()
Jérémie Galarneau [Wed, 11 Sep 2019 00:35:19 +0000 (20:35 -0400)] 
Fix: dereference of NULL pointer in stream_write()

stream_write() can be used with a NULL packet to write padding only to
a stream. In the case of a metadata stream, packet is assumed to
always be non-NULL which may not always be true.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoFix: report bytecode_push failure when pushing symbol
Jérémie Galarneau [Wed, 11 Sep 2019 00:26:47 +0000 (20:26 -0400)] 
Fix: report bytecode_push failure when pushing symbol

The last use of bytecode_push's return value is not checked in
visit_node_load_expression_legacy().

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoFix: only invoke PERROR() on failure to close sessiond_socket
Jérémie Galarneau [Wed, 11 Sep 2019 00:24:20 +0000 (20:24 -0400)] 
Fix: only invoke PERROR() on failure to close sessiond_socket

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoClean-up: lttng: check status returned when checking rotation state
Jérémie Galarneau [Wed, 11 Sep 2019 00:17:34 +0000 (20:17 -0400)] 
Clean-up: lttng: check status returned when checking rotation state

While the use of the destruction handle in the lttng client guarantees
that obtaining the rotation's state will succeed, it is a poor example
to give of using this API. Moreover, we don't wait to give the
impression that this could never change.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoFix: communication error unreported in relay_rotate_session_streams
Jérémie Galarneau [Wed, 11 Sep 2019 00:08:30 +0000 (20:08 -0400)] 
Fix: communication error unreported in relay_rotate_session_streams

'ret' is set to 0 even if the communication with the relay daemon
fails in relay_rotate_session_streams().

This also fixes a dead-assignment warning.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoFix: unreported error in relay_close_trace_chunk
Jérémie Galarneau [Wed, 11 Sep 2019 00:04:33 +0000 (20:04 -0400)] 
Fix: unreported error in relay_close_trace_chunk

Errors encountered in relay_close_trace_chunk are not reported to the
caller as the 'ret' value is re-used to check for the successful
'append' to a dynamic buffer. Use a separate variable.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoClean-up: remove dead assignment from ht cleanup thread launcher
Jérémie Galarneau [Wed, 11 Sep 2019 00:02:32 +0000 (20:02 -0400)] 
Clean-up: remove dead assignment from ht cleanup thread launcher

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoClean-up: remove dead assignment from thread_rotation
Jérémie Galarneau [Wed, 11 Sep 2019 00:01:32 +0000 (20:01 -0400)] 
Clean-up: remove dead assignment from thread_rotation

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 weeks agoClean-up: remove dead assignment in setup_channel_trace_path
Jérémie Galarneau [Tue, 10 Sep 2019 23:59:20 +0000 (19:59 -0400)] 
Clean-up: remove dead assignment in setup_channel_trace_path

setup_channel_trace_path() returns NULL on error. There is no
need to update a return code.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
This page took 0.097783 seconds and 4 git commands to generate.