Skip to content

Commit

Permalink
Fix: lttng: poptGetArg doesn't provide string ownership
Browse files Browse the repository at this point in the history 
The string returned by poptGetArg() is 'const' because it is owned by
the popt librairy and is free'd by it when poptFreeContext() is called.
Copy those strings when we need to alter them to maintain proper
ownership.

The latest release of popt (v1.19) introduced a breaking
change (changing the ownership of left-over command line arguments) that
can cause double free()-s.

This is ultimately due to this upstream commit in popt 1.19:
rpm-software-management/popt@7182e46

which is derived from a package patch:
https://src.fedoraproject.org/rpms/babeltrace/c/d48452beff87b145c038f070e7182358db04336c?branch=rawhide

Change-Id: Id2535d1534c0e47cc0747968d6dd60a587f0b810
Signed-off-by: Michael Jeanson <[email protected]>
Signed-off-by: Jérémie Galarneau <[email protected]>
  • Loading branch information
@mjeanson @jgalar
mjeanson authored and jgalar committed Jan 5, 2023
1 parent 8aec5b8 commit b7f4ea0
Show file tree
Hide file tree
Showing 15 changed files with 230 additions and 170 deletions.
25 changes: 13 additions & 12 deletions src/bin/lttng/commands/clear.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ int cmd_clear(int argc, const char **argv)
int ret = CMD_SUCCESS , i, command_ret = CMD_SUCCESS, success = 1;
static poptContext pc;
char *session_name = NULL;
const char *arg_session_name = NULL;
const char *leftover = NULL;
bool free_session_name = false;
struct lttng_session *sessions = NULL;
int count;
int found;
Expand Down Expand Up @@ -213,19 +213,22 @@ int cmd_clear(int argc, const char **argv)
}

if (!opt_clear_all) {
session_name = (char *) poptGetArg(pc);
if (!session_name) {
arg_session_name = poptGetArg(pc);
if (!arg_session_name) {
/* No session name specified, lookup default */
session_name = get_session_name();
} else {
session_name = strdup(arg_session_name);
if (session_name == NULL) {
command_ret = CMD_ERROR;
success = 0;
goto mi_closing;
PERROR("Failed to copy session name");
}
free_session_name = true;
}
} else {
session_name = NULL;

if (session_name == NULL) {
command_ret = CMD_ERROR;
success = 0;
goto mi_closing;
}
}

leftover = poptGetArg(pc);
Expand Down Expand Up @@ -307,9 +310,7 @@ int cmd_clear(int argc, const char **argv)
}

free(sessions);
if (free_session_name) {
free(session_name);
}
free(session_name);

/* Overwrite ret if an error occurred during clear_session/all */
ret = command_ret ? command_ret : ret;
Expand Down
40 changes: 21 additions & 19 deletions src/bin/lttng/commands/create.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#include <lttng/lttng.h>

static char *opt_output_path;
static char *opt_session_name;
static char *opt_url;
static char *opt_ctrl_url;
static char *opt_data_url;
Expand Down Expand Up @@ -131,7 +130,7 @@ static int mi_created_session(const char *session_name)
}

static
struct lttng_session_descriptor *create_session_descriptor(void)
struct lttng_session_descriptor *create_session_descriptor(const char *session_name)
{
ssize_t uri_count;
enum output_type output_type;
Expand Down Expand Up @@ -203,17 +202,17 @@ struct lttng_session_descriptor *create_session_descriptor(void)
case OUTPUT_UNSPECIFIED:
case OUTPUT_LOCAL:
descriptor = lttng_session_descriptor_snapshot_local_create(
opt_session_name,
session_name,
output_type == OUTPUT_LOCAL ?
local_output_path : NULL);
break;
case OUTPUT_NONE:
descriptor = lttng_session_descriptor_snapshot_create(
opt_session_name);
session_name);
break;
case OUTPUT_NETWORK:
descriptor = lttng_session_descriptor_snapshot_network_create(
opt_session_name, uri_str1, uri_str2);
session_name, uri_str1, uri_str2);
break;
default:
abort();
Expand All @@ -226,25 +225,25 @@ struct lttng_session_descriptor *create_session_descriptor(void)
goto end;
}
descriptor = lttng_session_descriptor_live_network_create(
opt_session_name, uri_str1, uri_str2,
session_name, uri_str1, uri_str2,
opt_live_timer);
} else {
/* Regular session. */
switch (output_type) {
case OUTPUT_UNSPECIFIED:
case OUTPUT_LOCAL:
descriptor = lttng_session_descriptor_local_create(
opt_session_name,
session_name,
output_type == OUTPUT_LOCAL ?
local_output_path : NULL);
break;
case OUTPUT_NONE:
descriptor = lttng_session_descriptor_create(
opt_session_name);
session_name);
break;
case OUTPUT_NETWORK:
descriptor = lttng_session_descriptor_network_create(
opt_session_name, uri_str1, uri_str2);
session_name, uri_str1, uri_str2);
break;
default:
abort();
Expand Down Expand Up @@ -281,7 +280,7 @@ struct lttng_session_descriptor *create_session_descriptor(void)
*
* Returns one of the CMD_* result constants.
*/
static int create_session(void)
static int create_session(const char *session_name)
{
int ret, i;
char shm_path[LTTNG_PATH_MAX] = {};
Expand All @@ -293,8 +292,8 @@ static int create_session(void)
const char *created_session_name;

/* Validate options. */
if (opt_session_name) {
if (strlen(opt_session_name) > NAME_MAX) {
if (session_name) {
if (strlen(session_name) > NAME_MAX) {
ERR("Session name too long. Length must be lower or equal to %d",
NAME_MAX);
ret = CMD_ERROR;
Expand All @@ -305,11 +304,11 @@ static int create_session(void)
* Both are reserved for the default session name. See bug #449 to
* understand why we need to check both here.
*/
if ((strncmp(opt_session_name, DEFAULT_SESSION_NAME "-",
if ((strncmp(session_name, DEFAULT_SESSION_NAME "-",
strlen(DEFAULT_SESSION_NAME) + 1) == 0) ||
(strncmp(opt_session_name, DEFAULT_SESSION_NAME,
(strncmp(session_name, DEFAULT_SESSION_NAME,
strlen(DEFAULT_SESSION_NAME)) == 0 &&
strlen(opt_session_name) == strlen(DEFAULT_SESSION_NAME))) {
strlen(session_name) == strlen(DEFAULT_SESSION_NAME))) {
ERR("%s is a reserved keyword for default session(s)",
DEFAULT_SESSION_NAME);
ret = CMD_ERROR;
Expand All @@ -329,7 +328,7 @@ static int create_session(void)
goto error;
}

session_descriptor = create_session_descriptor();
session_descriptor = create_session_descriptor(session_name);
if (!session_descriptor) {
ret = CMD_ERROR;
goto error;
Expand Down Expand Up @@ -375,7 +374,7 @@ static int create_session(void)
* An auto-generated session name already includes the creation
* timestamp.
*/
if (opt_session_name) {
if (session_name) {
uint64_t creation_time;
struct tm *timeinfo;
time_t creation_time_t;
Expand Down Expand Up @@ -655,6 +654,7 @@ int cmd_create(int argc, const char **argv)
{
int opt, ret = CMD_SUCCESS, command_ret = CMD_SUCCESS, success = 1;
char *opt_arg = NULL;
const char *arg_session_name = NULL;
const char *leftover = NULL;
static poptContext pc;

Expand Down Expand Up @@ -761,7 +761,9 @@ int cmd_create(int argc, const char **argv)
goto end;
}
}
opt_session_name = (char*) poptGetArg(pc);

/* Get the optional session name argument. */
arg_session_name = poptGetArg(pc);

leftover = poptGetArg(pc);
if (leftover) {
Expand All @@ -770,7 +772,7 @@ int cmd_create(int argc, const char **argv)
goto end;
}

command_ret = create_session();
command_ret = create_session(arg_session_name);
if (command_ret) {
success = 0;
}
Expand Down
25 changes: 13 additions & 12 deletions src/bin/lttng/commands/destroy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include <common/sessiond-comm/sessiond-comm.h>
#include <common/utils.h>

static char *opt_session_name;
static int opt_destroy_all;
static int opt_no_wait;

Expand Down Expand Up @@ -270,6 +269,7 @@ int cmd_destroy(int argc, const char **argv)
int ret = CMD_SUCCESS , i, command_ret = CMD_SUCCESS, success = 1;
static poptContext pc;
char *session_name = NULL;
const char *arg_session_name = NULL;
const char *leftover = NULL;

struct lttng_session *sessions = NULL;
Expand Down Expand Up @@ -342,18 +342,22 @@ int cmd_destroy(int argc, const char **argv)
success = 0;
}
} else {
opt_session_name = (char *) poptGetArg(pc);
arg_session_name = poptGetArg(pc);

if (!opt_session_name) {
if (!arg_session_name) {
/* No session name specified, lookup default */
session_name = get_session_name();
} else {
session_name = strdup(arg_session_name);
if (session_name == NULL) {
command_ret = CMD_ERROR;
success = 0;
goto mi_closing;
PERROR("Failed to copy session name");
}
} else {
session_name = opt_session_name;
}

if (session_name == NULL) {
command_ret = CMD_ERROR;
success = 0;
goto mi_closing;
}

/* Find the corresponding lttng_session struct */
Expand Down Expand Up @@ -419,10 +423,7 @@ int cmd_destroy(int argc, const char **argv)
ret = ret ? ret : -LTTNG_ERR_MI_IO_FAIL;
}

if (opt_session_name == NULL) {
free(session_name);
}

free(session_name);
free(sessions);

/* Overwrite ret if an error occurred during destroy_session/all */
Expand Down
24 changes: 17 additions & 7 deletions src/bin/lttng/commands/disable_channels.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

#include "../command.h"

static char *opt_channels;
static int opt_kernel;
static char *opt_session_name;
static int opt_userspace;
Expand Down Expand Up @@ -95,7 +94,7 @@ static int mi_partial_channel_print(char *channel_name, unsigned int enabled,
/*
* Disabling channel using the lttng API.
*/
static int disable_channels(char *session_name)
static int disable_channels(char *session_name, char *channel_list)
{
int ret = CMD_SUCCESS, warn = 0, success;

Expand Down Expand Up @@ -134,7 +133,7 @@ static int disable_channels(char *session_name)
}

/* Strip channel list */
channel_name = strtok(opt_channels, ",");
channel_name = strtok(channel_list, ",");
while (channel_name != NULL) {
DBG("Disabling channel %s", channel_name);

Expand Down Expand Up @@ -208,6 +207,8 @@ int cmd_disable_channels(int argc, const char **argv)
int opt, ret = CMD_SUCCESS, command_ret = CMD_SUCCESS, success = 1;
static poptContext pc;
char *session_name = NULL;
char *channel_list = NULL;
const char *arg_channel_list = NULL;
const char *leftover = NULL;

pc = poptGetContext(NULL, argc, argv, long_options, 0);
Expand Down Expand Up @@ -237,9 +238,16 @@ int cmd_disable_channels(int argc, const char **argv)
goto end;
}

opt_channels = (char*) poptGetArg(pc);
if (opt_channels == NULL) {
ERR("Missing channel name(s).\n");
arg_channel_list = poptGetArg(pc);
if (arg_channel_list == NULL) {
ERR("Missing channel name(s).");
ret = CMD_ERROR;
goto end;
}

channel_list = strdup(arg_channel_list);
if (channel_list == NULL) {
PERROR("Failed to copy channel name");
ret = CMD_ERROR;
goto end;
}
Expand Down Expand Up @@ -286,7 +294,7 @@ int cmd_disable_channels(int argc, const char **argv)
}
}

command_ret = disable_channels(session_name);
command_ret = disable_channels(session_name, channel_list);
if (command_ret) {
success = 0;
}
Expand Down Expand Up @@ -327,6 +335,8 @@ int cmd_disable_channels(int argc, const char **argv)
free(session_name);
}

free(channel_list);

/* Overwrite ret if an error occurred in disable_channels */
ret = command_ret ? command_ret : ret;

Expand Down
Loading

0 comments on commit b7f4ea0

Please sign in to comment.