Project

General

Profile

Actions

Bug #752

open

Output paths need better handling than truncation

Added by Daniel U. Thibault over 10 years ago. Updated over 8 years ago.

Status:
In Progress
Priority:
Low
Target version:
Start date:
03/07/2014
Due date:
% Done:

0%

Estimated time:

Description

For instance, in the aftermath of 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).

Actions #1

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.

Actions #2

Updated by Christian Babeux almost 10 years ago

  • Project changed from LTTng to LTTng-tools
Actions #3

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
Actions #4

Updated by Jérémie Galarneau about 9 years ago

  • Assignee changed from Jérémie Galarneau to Philippe Proulx
Actions #5

Updated by Philippe Proulx about 9 years ago

  • Target version changed from 2.7 to 2.8
Actions #6

Updated by Jérémie Galarneau over 8 years ago

  • Target version changed from 2.8 to 2.9
Actions

Also available in: Atom PDF