Bug #752
openOutput paths need better handling than truncation
0%
Description
lttng-tools/src/bin/lttng-sessiond/cmd.c:record_ust_snapshot
, the msg.u.snapshot_channel.pathname
is limited to PATH_MAX (typically 4096) but is built with "%s/%s-%s-%" PRIu64 "%s"
, where the successive arguments are (discounting the closing nulls):
output->consumer->dst.trace_path
PATH_MAX - 1 (no trailing /)- / 1
output->name
NAME_MAX - 1- - 1
output->datetime
15- - 1
output->nb_snapshot
20 digits (unsigned 64-bit integer)session_path
PATH_MAX - 1 (including leading and trailing /)
The worst-case session_path
part is '/ust/pid/<proc>-<vpid>-<datetime>/'
so it's actually limited to 12+15+5+15 = 47 characters (/proc/PID/status.name
is truncated to 15 characters, and VPID is unsigned 16-bit for 5 characters) (closing null excluded). So one solution would be to limit the consumer->dst.trace_path
to PATH_MAX - (NAME_MAX - 1 + 15 + 20 + 47 + 3) - 1 (for the null). However, if we want the path+filetitles of the channel files to fit in PATH_MAX, we need to chop another NAME_MAX off (and also limit channel names to NAME_MAX - (1 + 5 + 1 + 10 + 1) [underscore, 16-bit unsigned CPU ID, underscore, 32-bit unsigned chunk number, null] so they fit).
Truncation remains nevertheless possible, and would wreak havoc with the trace output tree. Babeltrace
and the user count on proper folder and file tree structure to manage their traces. The code needs to detect instances of truncation and report them as errors.
As an aside, the snapshot output name should be limited to MAX_PATH - (1+10) because it gets suffixed with a hyphen and an unsigned 32-bit integer (the output set sequential ID).
Updated by Daniel U. Thibault over 10 years ago
Here is a related string-length problem lurking in lttng-relayd
(lttng-tools/src/common/utils.c
):
int utils_mkdir_recursive(const char *path, mode_t mode) { char *p, tmp[PATH_MAX]; size_t len; int ret; assert(path); ret = snprintf(tmp, sizeof(tmp), "%s", path); if (ret < 0) { PERROR("snprintf mkdir"); goto error; } len = ret; if (tmp[len - 1] == '/') { tmp[len - 1] = 0; } [...]
The const char *path
received by this function from lttng-tools/src/bin/lttng-relayd/main.c
is an unlimited char *
received from the command line ('lttng-relayd --output=...'
). snprintf
won't write more than sizeof(tmp)
bytes (null included), which is fine, but when the const char *path
is larger than char tmp[PATH_MAX]
, snprintf
returns the number of characters (null excluded) needed to write "%s", path
. Thus, a return value of size or more means that the output was truncated. utils_mkdir_recursive
will then try to read a byte somewhere beyond the end of char tmp[PATH_MAX]
, and maybe even change it.
utils_mkdir_recursive
should check for ret >= sizeof(tmp)
and treat it as an error.
There may be other places where utils_mkdir_recursive
is called with a potentially overlong path
argument.
Updated by Christian Babeux almost 10 years ago
- Project changed from LTTng to LTTng-tools
Updated by Jérémie Galarneau about 9 years ago
- Status changed from New to In Progress
- Assignee set to Jérémie Galarneau
- Target version changed from 2.5 to 2.7
Updated by Jérémie Galarneau about 9 years ago
- Assignee changed from Jérémie Galarneau to Philippe Proulx
Updated by Philippe Proulx about 9 years ago
- Target version changed from 2.7 to 2.8
Updated by Jérémie Galarneau over 8 years ago
- Target version changed from 2.8 to 2.9