lttng-tools.git
4 years agoCleanup: remove unused internal lttng_session_descriptor_get_base_path
Jonathan Rajotte [Fri, 25 Oct 2019 22:12:02 +0000 (18:12 -0400)] 
Cleanup: remove unused internal lttng_session_descriptor_get_base_path

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoRefactor: Move set session path to own function
Jonathan Rajotte [Fri, 25 Oct 2019 22:12:01 +0000 (18:12 -0400)] 
Refactor: Move set session path to own function

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: move set base_path of session to URI configuration
Jonathan Rajotte [Fri, 25 Oct 2019 22:12:00 +0000 (18:12 -0400)] 
Fix: move set base_path of session to URI configuration

The load code still uses the "old" API to create and configure network
session output (lttng_create_session followed by
lttng_set_consumer_url). The session base_path is only set in the
cmd_create_session_from_descriptor function. This results in invalid
network output paths when using a loaded session configuration (xml).

While we might want to move the load code to the new API, there is a
case to be made in not breaking the previous API behaviour.

To restore the expected behaviour of lttng_set_consumer_url, move the
assignation of the session base_path to the cmd_set_consumer_uri
function (LTTNG_SET_CONSUMER_URI).

Both the previous and session descriptor based creation API uses this
code path when needed (network output).

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: tests: re-add link to urcu-bp for _LGPL_SOURCE tests
Michael Jeanson [Tue, 5 Nov 2019 16:30:24 +0000 (11:30 -0500)] 
Fix: tests: re-add link to urcu-bp for _LGPL_SOURCE tests

Tests with tracepoints defined with _LGPL_SOURCE require to be
explicitly linked against urcu-bp.

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: tests: use DL_LIBS variable in ust multi-lib test
Michael Jeanson [Thu, 24 Oct 2019 15:36:33 +0000 (11:36 -0400)] 
Fix: tests: use DL_LIBS variable in ust multi-lib test

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: lttng: initialize sessions pointer to NULL
Jonathan Rajotte [Fri, 25 Oct 2019 21:56:26 +0000 (17:56 -0400)] 
Fix: lttng: initialize sessions pointer to NULL

lttng_list_sessions does not set the passed pointer to NULL on empty
return. This leads to a deallocation of an invalid pointer (segfault).

For returns of size 0, the value of the passed argument should be
considered "undefined".

Refactor error handling a bit by removing the "error" jump. Always
call free on the 'sessions' object.

Fixes #1205

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoUse pkgconfig to detect and configure liblttng-ust
Michael Jeanson [Thu, 24 Oct 2019 15:36:22 +0000 (11:36 -0400)] 
Use pkgconfig to detect and configure liblttng-ust

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: check for dtrace and sdt.h before enabling SDT uprobe tests
Michael Jeanson [Thu, 31 Oct 2019 20:12:46 +0000 (16:12 -0400)] 
Fix: check for dtrace and sdt.h before enabling SDT uprobe tests

Add a configure switch '--enable-sdt-uprobe / --disable-sdt-uprobe', the
default behavior of enabling the test if the requirements are found is
kept but it's now possible to explicitly disable it.

Also add the detection of the dtrace binary and its override trough the
DTRACE environment variable.

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: consumerd: crash occurs when taking snapshot of ust channel
Jérémie Galarneau [Wed, 30 Oct 2019 19:35:28 +0000 (15:35 -0400)] 
Fix: consumerd: crash occurs when taking snapshot of ust channel

Commit 8e1ef46e8 added an acquisition of the metadata_stream's lock
during consumer_metadata_cache_flushed() as stream attributes are
used. However, when this function is called, the metadata channel's
stream can already be NULL, as indicated by the function's comments.

Check if the stream is NULL before attempting to acquire its lock.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: trace-chunk: log the cause of file open failures
Jérémie Galarneau [Tue, 29 Oct 2019 19:57:59 +0000 (15:57 -0400)] 
Fix: trace-chunk: log the cause of file open failures

Use the PERROR macro instead of ERR to obtain the "errno" message
when an error occurs while opening a file relative to a trace
chunk.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: relayd: live: crash when creating viewer streams
Jérémie Galarneau [Tue, 29 Oct 2019 03:57:01 +0000 (23:57 -0400)] 
Fix: relayd: live: crash when creating viewer streams

Viewer streams can be creating while serving a "GET_STREAMS" viewer
client command for a session that is being destroyed. If this happens,
the viewer streams will be created with a NULL viewer trace chunk,
which would result in a crash.

The fix consists in returning a stream error when such a command
happens during the destruction of a session. This is the same
behaviour than if the session could not be found at all, introducing
no meaningful change in behaviour from the viewer's perspective.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: relayd: live: crash on attach to a session without trace chunk
Jérémie Galarneau [Tue, 29 Oct 2019 03:32:36 +0000 (23:32 -0400)] 
Fix: relayd: live: crash on attach to a session without trace chunk

Attaching to a session that doesn't have a current trace chunk results
in a crash when the viewer streams are created from a NULL viewer
trace chunk.

Live clients are prevented from attaching to sessions without a
current trace chunk as those sessions are either being destroyed or
too young to have a trace chunk, meaning that they don't have streams
yet. Live clients will receive the "unknown" status code that they
already receive when asking an unknown session. Since such sessions
are not listed, this shouldn't change any exposed behaviour.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: relayd: live: some listed sessions are not attacheable
Jérémie Galarneau [Tue, 29 Oct 2019 03:29:53 +0000 (23:29 -0400)] 
Fix: relayd: live: some listed sessions are not attacheable

The list sessions command currently returns sessions that do not
have a current trace chunk. This can be caused by the session
either being destroyed or being so young that it hasn't had a
trace chunk created against it yet.

In both cases, such sessions would not be attacheable in their
current condition. This fix omits them from from the listing
to reduce the number of failures at the "attach session" and
"attach stream" step.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: relayd: don't put un-acquired trace chunk reference
Jérémie Galarneau [Mon, 28 Oct 2019 20:53:48 +0000 (16:53 -0400)] 
Fix: relayd: don't put un-acquired trace chunk reference

stream_create() should not release (put) the reference to the current
trace chunk in its error path if it could not acquire a new reference
in the first place.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: relayd: don't put un-acquired viewer trace chunk reference
Jérémie Galarneau [Mon, 28 Oct 2019 20:26:48 +0000 (16:26 -0400)] 
Fix: relayd: don't put un-acquired viewer trace chunk reference

viewer_stream_create() should not release (put) the reference to
the viewer_trace_chunk in its error path if it could not acquire
a new reference in the first place.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: consumerd: NULL pointer dereference during metadata sync
Jérémie Galarneau [Mon, 28 Oct 2019 19:37:34 +0000 (15:37 -0400)] 
Fix: consumerd: NULL pointer dereference during metadata sync

The following crash was reported when short-lived applications
are traced in a live session with per-pid buffering channels.

From the original report:

```
 Thread 1 (Thread 0x7f72b67fc700 (LWP 1912155)):
 #0  0x00005650b3f6ccbd in commit_one_metadata_packet (stream=0x7f729c010bf0) at ust-consumer.c:2537
 #1  0x00005650b3f6cf58 in lttng_ustconsumer_sync_metadata (ctx=0x5650b588ce60, metadata=0x7f729c010bf0) at ust-consumer.c:2608
 #2  0x00005650b3f4dba3 in do_sync_metadata (metadata=0x7f729c010bf0, ctx=0x5650b588ce60) at consumer-stream.c:471
 #3  0x00005650b3f4dd3c in consumer_stream_sync_metadata (ctx=0x5650b588ce60, session_id=0) at consumer-stream.c:548
 #4  0x00005650b3f6de78 in lttng_ustconsumer_read_subbuffer (stream=0x7f729c0058e0, ctx=0x5650b588ce60) at ust-consumer.c:2917
 #5  0x00005650b3f45196 in lttng_consumer_read_subbuffer (stream=0x7f729c0058e0, ctx=0x5650b588ce60) at consumer.c:3524
 #6  0x00005650b3f42da7 in consumer_thread_data_poll (data=0x5650b588ce60) at consumer.c:2894
 #7  0x00007f72bdc476db in start_thread (arg=0x7f72b67fc700) at pthread_create.c:463
 #8  0x00007f72bd97088f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

The segfault happen on the access to 'stream->chan->metadata_cache->lock'
chan value here is zero.
```

The problem is easily reproducible if a sleep(1) is added just after
the call to lttng_ustconsumer_request_metadata(), before the metadata
stream lock is re-acquired.

During the execution of the "request_metadata", an application can
close. This will cause the session daemon to push any remaining
metadata to the consumer daemon and to close the metadata channel.

Closing the metadata channel closes the metadata stream's wait_fd,
which is an internal pipe. The closure of the metadata pipe is
detected by the metadata_poll thread, which will ensure that all
metadata has been consumed before issuing the deletion of the metadata
stream and channel.

During the deletion, the channel's "stream" attribute the stream's
"chan" attribute are set to NULL as both are logically deleted and
should not longer be used.

Meanwhile, the thread executing commit_one_metadata_packet()
re-acquires the metadata stream lock and trips on the now-NULL "chan"
member.

The fix consists in checking if the metadata stream is logically
deleted after its lock is re-acquired. It is correct for the
sync_metadata operation to then complete successfully as the metadata
is synced: the metadata guarantees this before deleting the
stream/channel.

Since the metadata stream's lifetime is protected by its lock, there
may be other sites that need such a check. The lock and deletion check
could be combined into a single consumer_stream_lock() helper in
follow-up fixes.

Reported-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoconsumerd: clean-up: stream attribute accessed without locking stream
Jérémie Galarneau [Mon, 28 Oct 2019 18:52:44 +0000 (14:52 -0400)] 
consumerd: clean-up: stream attribute accessed without locking stream

consumer_metadata_cache_flushed makes use of the metadata stream's
ust_metadata_pushed attribute without locking while it is updated by
commit_one_metadata_packet() which holds the metadata stream lock.

This is marked as a clean-up since the attribute appears to always be
accessed while the metadata cache lock is held. However this is a
_channel_ attribute and the stream and channel lifetimes do not match,
making the locking assumptions conceptually dubious.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoTests: namespace tests fail to build on older libc
Jérémie Galarneau [Thu, 24 Oct 2019 21:06:17 +0000 (17:06 -0400)] 
Tests: namespace tests fail to build on older libc

The namespace tests fail to build on older libcs that don't define the
various namespace clone flags. To circumvent this problem, local
definitions from Linux's sched.h are added.

The kernel's support of the various namespaces is tested by the test
runner. Hence, it is not a problem to build these test application
against a kernel that isn't configured to support (some of) those
namespaces; the applications are not invoked for unsupported
namespaces.

Acked-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: check for lttng-ust >= 2.11 at configure
Michael Jeanson [Wed, 23 Oct 2019 15:08:34 +0000 (11:08 -0400)] 
Fix: check for lttng-ust >= 2.11 at configure

We don't support building lttng-tools against an older version of
lttng-ust, make this check explicitly at configure.

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agosessiond: build fails in --without-lttng-ust configuration
Jérémie Galarneau [Thu, 24 Oct 2019 00:27:12 +0000 (20:27 -0400)] 
sessiond: build fails in --without-lttng-ust configuration

The include of macros.h was changed when syncing with the latest
usb-abi.h header and caused the build to fail in the
--without-lttng-ust configuration.

Changed the include to refer to common/macros.h.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoTests: use "kill -0" for app existence check in NS tests
Jonathan Rajotte [Tue, 15 Oct 2019 20:50:46 +0000 (16:50 -0400)] 
Tests: use "kill -0" for app existence check in NS tests

Removing the sleep 0.5 we hit a race where the "-f /proc/$pid_app"
test returns false immediately. Using "kill -0" protects us against
such races. From kill (2):

If sig is 0, then no signal is sent, but existence and permission
checks are still performed; this can be used to check for the
existence of a process ID or process group ID that the caller is
permitted to signal.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoTests: add kernel namespace context change tests
Michael Jeanson [Tue, 2 Apr 2019 17:38:07 +0000 (13:38 -0400)] 
Tests: add kernel namespace context change tests

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoTests: add UST namespace context change tests
Michael Jeanson [Mon, 1 Apr 2019 15:53:57 +0000 (11:53 -0400)] 
Tests: add UST namespace context change tests

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoTests: add kernel namespace contexts tests
Michael Jeanson [Mon, 1 Apr 2019 15:53:20 +0000 (11:53 -0400)] 
Tests: add kernel namespace contexts tests

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoTests: add UST namespace contexts tests
Michael Jeanson [Mon, 1 Apr 2019 15:51:52 +0000 (11:51 -0400)] 
Tests: add UST namespace contexts tests

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoMI: add support for namespace and uid/gid contexts
Jonathan Rajotte [Tue, 2 Apr 2019 21:51:01 +0000 (17:51 -0400)] 
MI: add support for namespace and uid/gid contexts

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>
4 years agoAdd UST uid/gid contexts
Michael Jeanson [Tue, 12 Feb 2019 16:51:55 +0000 (11:51 -0500)] 
Add UST uid/gid contexts

Add the following userspace tracer context fields:
  - vuid
    Virtual real user ID: real user ID as seen from the point of view of
    the current user namespace

  - veuid
    Virtual effective user ID: effective user ID as seen from the point of
    view of the current user namespace

  - vsuid
    Virtual saved set-user ID: saved set-user ID as seen from the point of
    view of the current user namespace

  - vgid
    Virtual real group ID: real group ID as seen from the point of view of
    the current user namespace

  - vegid
    Virtual effective group ID: effective group ID as seen from the point of
    view of the current user namespace

  - vsgid
    Virtual saved set-group ID: saved set-group ID as seen from the point of
    view of the current user namespace

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoAdd kernel uid/gid contexts
Michael Jeanson [Tue, 12 Feb 2019 16:51:42 +0000 (11:51 -0500)] 
Add kernel uid/gid contexts

Add the following kernel tracer context fields:
  - uid
    Real user ID

  - euid
    Effective user ID

  - uid
    Real user ID

  - suid
    Saved set-user ID

  - gid
    Real group ID

  - egid
    Effective group ID

  - sgid
    Effective saved set-user group ID

  - vuid
    Virtual real user ID: real user ID as seen from the point of view of
    the current user namespace

  - veuid
    Virtual effective user ID: effective user ID as seen from the point of
    view of the current user namespace

  - vsuid
    Virtual saved set-user ID: saved set-user ID as seen from the point of
    view of the current user namespace

  - vgid
    Virtual real group ID: real group ID as seen from the point of view of
    the current user namespace

  - vegid
    Virtual effective group ID: effective group ID as seen from the point of
    view of the current user namespace

  - vsgid
    Virtual saved set-group ID: saved set-group ID as seen from the point of
    view of the current user namespace

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoAdd UST namespace contexts
Michael Jeanson [Tue, 12 Feb 2019 16:51:21 +0000 (11:51 -0500)] 
Add UST namespace contexts

Add the following userspace namespace contexts:
  - cgroup_ns
    Cgroup root directory namespace: inode number of the current
    cgroup namespace in the proc filesystem.

  - ipc_ns
    System V IPC, POSIX message queues namespace: inode number of the
    current IPC namespace in the proc filesystem.

  - mnt_ns
    Mount points namespace: inode number of the current mount
    namespace in the proc filesystem.

  - net_ns
    Network devices, stacks, ports namespace: inode number of the
    current network namespace in the proc filesystem.

  - pid_ns
    Process IDs namespace: inode number of the current pid namespace
    in the proc filesystem.

  - user_ns
    User and group IDs namespace: inode number of the current user
    namespace in the proc filesystem.

  - uts_ns
    Hostname and NIS domain name namespace: inode number of the
    current uts namespace in the proc filesystem.

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoAdd kernel namespace contexts
Michael Jeanson [Tue, 12 Feb 2019 16:51:05 +0000 (11:51 -0500)] 
Add kernel namespace contexts

Add the following kernel namespace contexts:

  - cgroup_ns
    Cgroup root directory namespace: inode number of the current
    cgroup namespace in the proc filesystem.

  - ipc_ns
    System V IPC, POSIX message queues namespace: inode number of the
    current IPC namespace in the proc filesystem.

  - mnt_ns
    Mount points namespace: inode number of the current mount
    namespace in the proc filesystem.

  - net_ns
    Network devices, stacks, ports namespace: inode number of the
    current network namespace in the proc filesystem.

  - pid_ns
    Process IDs namespace: inode number of the current pid namespace
    in the proc filesystem.

  - user_ns
    User and group IDs namespace: inode number of the current user
    namespace in the proc filesystem.

  - uts_ns
    Hostname and NIS domain name namespace: inode number of the
    current uts namespace in the proc filesystem.

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoUpdate local copy of ust-abi.h to reflect addition of ns contexts
Jérémie Galarneau [Tue, 22 Oct 2019 20:41:18 +0000 (16:41 -0400)] 
Update local copy of ust-abi.h to reflect addition of ns contexts

New contexts were added to LTTng-UST and will be used by
LTTng-Tools. Re-sync this LTTng-UST header to ensure the build
doesn't break in the --without-lttng-ust configuration.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoTests: fix: tmp dir can be a symlink
Jonathan Rajotte [Tue, 22 Oct 2019 16:05:28 +0000 (12:05 -0400)] 
Tests: fix: tmp dir can be a symlink

Get the real path to perform valid comparison.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoDocs: relayd: document LTTNG_RELAYD_WORKING_DIRECTORY env variable
Jonathan Rajotte [Tue, 22 May 2018 18:33:37 +0000 (14:33 -0400)] 
Docs: relayd: document LTTNG_RELAYD_WORKING_DIRECTORY env variable

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoDocs: relayd: document the --working-directory/-w option in man page
Jonathan Rajotte [Tue, 22 May 2018 18:25:32 +0000 (14:25 -0400)] 
Docs: relayd: document the --working-directory/-w option in man page

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoIntroduce LTTNG_RELAYD_WORKING_DIRECTORY environment variable
Jonathan Rajotte [Tue, 22 May 2018 17:48:07 +0000 (13:48 -0400)] 
Introduce LTTNG_RELAYD_WORKING_DIRECTORY environment variable

LTTNG_RELAYD_WORKING_DIRECTORY is equivalent to the --working-directory
command line options.

Note: when using --working-directory, the command line option always
overwrite the environment configuration, LTTNG_RELAYD_WORKING_DIRECTORY
in this case.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoTests: add an lttng-relayd working directory test
Jonathan Rajotte [Fri, 18 May 2018 20:24:04 +0000 (16:24 -0400)] 
Tests: add an lttng-relayd working directory test

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agorelayd: introduce --working-directory/-w options
Jonathan Rajotte [Wed, 16 May 2018 22:24:01 +0000 (18:24 -0400)] 
relayd: introduce --working-directory/-w options

This new option allows the user to specify the working directory (CWD) of the
lttng-relayd process. This is especially useful when lttng-relayd is
started in daemon mode (-d) because by default the CWD is set to "/".
This can help control where coredumps, if any, get generated.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: compile fails for x32 arch
Kai Kang [Thu, 13 Dec 2018 02:55:36 +0000 (10:55 +0800)] 
Fix: compile fails for x32 arch

LTTng-Tools fails to compile for the x32 ABI:

| .../src/common/utils.c: Assembler messages:
| .../src/common/utils.c:1026: Error: register type mismatch for `bsr'
| .../src/common/utils.c:1028: Error: operand type mismatch for `movq'

Add a macro to only use the x86_64 inline assembly version of fls_u64()
when the LP64 ABI is used.

Signed-off-by: Kai Kang <kai.kang@windriver.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoTypo: occured -> occurred
Michael Jeanson [Fri, 18 Oct 2019 21:40:06 +0000 (17:40 -0400)] 
Typo: occured -> occurred

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix typo 'Attemp' -> 'Attempt'
Michael Jeanson [Fri, 18 Oct 2019 21:12:22 +0000 (17:12 -0400)] 
Fix typo 'Attemp' -> 'Attempt'

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: sessiond: use system LTTng-UST headers when available
Jérémie Galarneau [Fri, 18 Oct 2019 20:39:00 +0000 (16:39 -0400)] 
Fix: sessiond: use system LTTng-UST headers when available

The LTTng-Tools tree includes a local copy of three LTTng-UST headers:
  * ust-error.h
  * ust-ctl.h
  * ust-abi.h

The system headers should be used when UST support is configured to
ensure the appropriate ABI definitions are used. The local copies of
the headers should only be used when LTTng-Tools is built with the
--without-lttng-ust configuration option. Those headers are needed
since some UST support code is compiled-in even though the support
is deactivated.

A misconfiguration in the CI setup allowed us to notice that
sessiond-config.c is using the internal header unconditionally.

To ensure this doesn't happen in the future, the local copies
are renamed:
  * ust-error.h -> ust-error-internal.h
  * ust-ctl.h   -> ust-ctl-internal.h
  * ust-abi.h   -> ust-abi-internal.h

All code should use the `lttng-` prefixed versions of the headers
which include either the local or "system" copy of the headers
depending on the build configuration.

Reported-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agodoc/man: use specific revision date for each manual page
Philippe Proulx [Fri, 18 Oct 2019 19:53:05 +0000 (15:53 -0400)] 
doc/man: use specific revision date for each manual page

This patch makes each manual page indicate its own revision date with
the `revdate` AsciiDoc attribute.

In `asciidoc.conf`, we use this attribute to specify the DocBook
reference page date (see
<https://tdg.docbook.org/tdg/4.5/refentryinfo.html> and
<https://tdg.docbook.org/tdg/4.5/date.html>).

Without the DocBook date tag, `xmlto` uses the current date. You can
see this date at the bottom of the rendered manual page:

    ...

    SEE ALSO
           lttng-enable-rotation(1), lttng-disable-rotation(1), lttng(1)

    LTTng 2.12.0-pre             10/18/2019              LTTNG-ROTATE(1)

Using the manual page generation date seems unexpected for the reader
here.

For this initial change, I used the last commit date for each source
file.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agolttng-rotate.1.txt: update voice and document the `archives` subdir.
Philippe Proulx [Fri, 18 Oct 2019 19:29:31 +0000 (15:29 -0400)] 
lttng-rotate.1.txt: update voice and document the `archives` subdir.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: sessiond: unbalanced health register/unregister on error
Jérémie Galarneau [Thu, 17 Oct 2019 18:37:37 +0000 (14:37 -0400)] 
Fix: sessiond: unbalanced health register/unregister on error

A number of threads do not correctly pair registrations and
unregistrations to the health monitoring subsystem when an error
forces them to abort early. Since the pattern is mostly the same
in the notification and rotation thread, they are both fixed in
the same commit.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: sessiond: NULL thread_state provided to pthread_cleanup callback
Jérémie Galarneau [Thu, 17 Oct 2019 18:35:34 +0000 (14:35 -0400)] 
Fix: sessiond: NULL thread_state provided to pthread_cleanup callback

The callback registered through pthread_cleanup_push(...),
thread_init_cleanup, now expects a non-NULL thread_state argument.

The thread_state is passed to match this recent change.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: sessiond: leak of trace chunk on destruction error
Jérémie Galarneau [Wed, 16 Oct 2019 22:53:47 +0000 (18:53 -0400)] 
Fix: sessiond: leak of trace chunk on destruction error

By design, a trace chunk can be leaked on the consumer daemon's end if
the session daemon does not close it. This is because the consumer
daemon has no "top-level" session object which could bound the
lifetime of a trace chunk.

It was reported that errors during a session destruction operation
could result in a trace chunk leak being reported by the consumer
daemon on shut down.

In the case that was reported, the failure to launch an application
caused the metadata channel to never be created. When the session was
destroyed, the rotation of the metadata channel failed with a "channel
does not exist" error. This error caused cmd_rotate_session() to abort
before the trace chunk close command was sent to the consumer
daemon(s). This ultimately results in the leak described earlier.

The fix consists in performing the trace chunk close command on the
consumer daemon even if the rotation itself fails.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agocommon: cleanup error message mentioning mkdir
Jérémie Galarneau [Wed, 16 Oct 2019 22:25:57 +0000 (18:25 -0400)] 
common: cleanup error message mentioning mkdir

While there is a good chance that mkdir is the actual syscall that
fails when this error code is returned, the error messages should
describe the operation that failed and not its implementation.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: sessiond: session destruction errors are unreported
Jérémie Galarneau [Wed, 16 Oct 2019 22:22:32 +0000 (18:22 -0400)] 
Fix: sessiond: session destruction errors are unreported

The session daemon does not report errors which occur while setting-up
a session's destruction. For instance, if the implicit rotation or
rotation to the "null" chunk fails. While the session will be
destroyed (it will no longer appear in session listings), the session
daemon could have failed to destroy it properly and it could be
corrupted/unreadable.

This reports those errors so the user does not expect the session to
be readable (but it _could_ be).

This was discovered while investigating another, unrelated, issue.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: consumer: double unlock of rcu read lock on error
Jérémie Galarneau [Wed, 16 Oct 2019 16:35:23 +0000 (12:35 -0400)] 
Fix: consumer: double unlock of rcu read lock on error

Commit 6b584c2e changed the implementation of the "trace_chunk_exists"
command to use the then-new "exists" method of the trace chunk
registry. Before this change, the trace chunks were looked-up to check
for their existence.

Since the "exists" method doesn't require the caller to hold the rcu
read lock, it is acquired later on in the function. However, the rcu
read lock is still released on error when this function fails,
resulting in a double-unlock.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: sessiond: application channel creation failure stops start cmd
Jérémie Galarneau [Tue, 15 Oct 2019 20:56:22 +0000 (16:56 -0400)] 
Fix: sessiond: application channel creation failure stops start cmd

The creation of an application's channel can fail when, for instance,
a context can't be created. This causes applications that would have
been started _after_ it to never be started.

This keeps the iteration going on error and starts all applications
that could be started. This is more in line with the behaviour of
2.10 (and earlier) since those channel creations would occur as
applications registered and not on tracing "start".

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agosessiond: clean-up: enhance logging on event allocation failure
Jérémie Galarneau [Fri, 11 Oct 2019 21:12:26 +0000 (17:12 -0400)] 
sessiond: clean-up: enhance logging on event allocation failure

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agoFix: sessiond: don't assert on event creation error
Jérémie Galarneau [Fri, 11 Oct 2019 20:26:29 +0000 (16:26 -0400)] 
Fix: sessiond: don't assert on event creation error

Don't assert if an application tracer reports that an event already
exists. This could be caused by a bug on the tracer end or memory
corruption on the application's end. In either case, an assert() is
too strict; simply report the error.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years agosessiond: clean-up: typo in ust-app.c comment
Jérémie Galarneau [Fri, 11 Oct 2019 20:08:24 +0000 (16:08 -0400)] 
sessiond: clean-up: typo in ust-app.c comment

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 years ago.gitignore: ignore vscode files
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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
4 years 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>
This page took 0.05056 seconds and 5 git commands to generate.