From 99497cd03fb27fd2416a88d80292a0cc5647bff2 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 8 Aug 2011 20:12:46 -0400 Subject: [PATCH] Fix all strncpy() usage: need to set a final \0 character at the end strncpy does not necessarily set the last character to the null terminating character. It must be done explicitly in case of overflow. Signed-off-by: Mathieu Desnoyers --- liblttkconsumerd/liblttkconsumerd.c | 1 + liblttngctl/liblttngctl.c | 33 +++++++++++++------------ liblttsessiondcomm/liblttsessiondcomm.c | 4 ++- ltt-sessiond/kernel-ctl.c | 3 ++- ltt-sessiond/main.c | 6 +++++ ltt-sessiond/trace.c | 4 +++ ltt-sessiond/ust-ctl.c | 3 ++- lttng/commands/disable_events.c | 1 + lttng/commands/enable_events.c | 4 +++ 9 files changed, 40 insertions(+), 19 deletions(-) diff --git a/liblttkconsumerd/liblttkconsumerd.c b/liblttkconsumerd/liblttkconsumerd.c index a135570fd..9ad380c8d 100644 --- a/liblttkconsumerd/liblttkconsumerd.c +++ b/liblttkconsumerd/liblttkconsumerd.c @@ -177,6 +177,7 @@ static int kconsumerd_add_fd(struct lttcomm_kconsumerd_msg *buf, int consumerd_f tmp_fd->state = buf->state; tmp_fd->max_sb_size = buf->max_sb_size; strncpy(tmp_fd->path_name, buf->path_name, PATH_MAX); + tmp_fd->path_name[PATH_MAX - 1] = '\0'; /* Opening the tracefile in write mode */ ret = open(tmp_fd->path_name, diff --git a/liblttngctl/liblttngctl.c b/liblttngctl/liblttngctl.c index de1821032..ec01859a9 100644 --- a/liblttngctl/liblttngctl.c +++ b/liblttngctl/liblttngctl.c @@ -46,6 +46,18 @@ static struct lttcomm_lttng_msg llm; static char *tracing_group; static int connected; +/* + * Copy string from src to dst and enforce null terminated byte. + */ +static void copy_string(char *dst, const char *src, size_t len) +{ + if (src && dst) { + strncpy(dst, src, len); + /* Enforce the NULL terminated byte */ + dst[len - 1] = '\0'; + } +} + /* * send_data_sessiond * @@ -154,13 +166,14 @@ static int set_session_daemon_path(void) /* Are we in the tracing group ? */ ret = check_tracing_group(tracing_group); if (ret < 0 && getuid() != 0) { - if (sprintf(sessiond_sock_path, DEFAULT_HOME_CLIENT_UNIX_SOCK, - getenv("HOME")) < 0) { + if (snprintf(sessiond_sock_path, PATH_MAX, + DEFAULT_HOME_CLIENT_UNIX_SOCK, + getenv("HOME")) < 0) { return -ENOMEM; } } else { - strncpy(sessiond_sock_path, DEFAULT_GLOBAL_CLIENT_UNIX_SOCK, - sizeof(DEFAULT_GLOBAL_CLIENT_UNIX_SOCK)); + copy_string(sessiond_sock_path, DEFAULT_GLOBAL_CLIENT_UNIX_SOCK, + PATH_MAX); } return 0; @@ -300,18 +313,6 @@ static void copy_lttng_domain(struct lttng_domain *dom) } } -/* - * Copy string from src to dst and enforce null terminated byte. - */ -static void copy_string(char *dst, const char *src, size_t len) -{ - if (src && dst) { - strncpy(dst, src, len); - /* Enforce the NULL terminated byte */ - dst[len - 1] = '\0'; - } -} - /* * Start tracing for all trace of the session. */ diff --git a/liblttsessiondcomm/liblttsessiondcomm.c b/liblttsessiondcomm/liblttsessiondcomm.c index 296782401..cecd89d6a 100644 --- a/liblttsessiondcomm/liblttsessiondcomm.c +++ b/liblttsessiondcomm/liblttsessiondcomm.c @@ -127,6 +127,7 @@ int lttcomm_connect_unix_sock(const char *pathname) memset(&sun, 0, sizeof(sun)); sun.sun_family = AF_UNIX; strncpy(sun.sun_path, pathname, sizeof(sun.sun_path)); + sun.sun_path[sizeof(sun.sun_path) - 1] = '\0'; ret = connect(fd, (struct sockaddr *) &sun, sizeof(sun)); if (ret < 0) { @@ -191,7 +192,8 @@ int lttcomm_create_unix_sock(const char *pathname) memset(&sun, 0, sizeof(sun)); sun.sun_family = AF_UNIX; - strncpy(sun.sun_path, pathname, strlen(pathname)); + strncpy(sun.sun_path, pathname, sizeof(sun.sun_path)); + sun.sun_path[sizeof(sun.sun_path) - 1] = '\0'; /* Unlink the old file if present */ (void) unlink(pathname); diff --git a/ltt-sessiond/kernel-ctl.c b/ltt-sessiond/kernel-ctl.c index 407dd5804..419d1af7b 100644 --- a/ltt-sessiond/kernel-ctl.c +++ b/ltt-sessiond/kernel-ctl.c @@ -609,7 +609,8 @@ ssize_t kernel_list_events(int tracer_fd, struct lttng_event **events) goto error; } } - strncpy(elist[count].name, event, strlen(event)); + strncpy(elist[count].name, event, LTTNG_SYMBOL_NAME_LEN); + elist[count].name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0'; count++; } diff --git a/ltt-sessiond/main.c b/ltt-sessiond/main.c index 64493521b..36d4423ca 100644 --- a/ltt-sessiond/main.c +++ b/ltt-sessiond/main.c @@ -289,6 +289,7 @@ static int send_kconsumerd_channel_fds(int sock, struct ltt_kernel_channel *chan lkm.state = stream->state; lkm.max_sb_size = channel->channel->attr.subbuf_size; strncpy(lkm.path_name, stream->pathname, PATH_MAX); + lkm.path_name[PATH_MAX - 1] = '\0'; DBG("Sending fd %d to kconsumerd", lkm.fd); @@ -338,6 +339,7 @@ static int send_kconsumerd_fds(int sock, struct ltt_kernel_session *session) lkm.state = ACTIVE_FD; lkm.max_sb_size = session->metadata->conf->attr.subbuf_size; strncpy(lkm.path_name, session->metadata->pathname, PATH_MAX); + lkm.path_name[PATH_MAX - 1] = '\0'; ret = lttcomm_send_fds_unix_sock(sock, &lkm, &lkm.fd, 1, sizeof(lkm)); if (ret < 0) { @@ -1275,7 +1277,9 @@ static void list_lttng_sessions(struct lttng_session *sessions) */ cds_list_for_each_entry(session, &session_list_ptr->head, list) { strncpy(sessions[i].path, session->path, PATH_MAX); + sessions[i].path[PATH_MAX - 1] = '\0'; strncpy(sessions[i].name, session->name, NAME_MAX); + sessions[i].name[NAME_MAX - 1] = '\0'; i++; } } @@ -1321,6 +1325,7 @@ static void list_lttng_events(struct ltt_kernel_channel *kchan, /* Kernel channels */ cds_list_for_each_entry(event, &kchan->events_list.head , list) { strncpy(events[i].name, event->event->name, LTTNG_SYMBOL_NAME_LEN); + events[i].name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0'; events[i].enabled = event->enabled; switch (event->event->instrumentation) { case LTTNG_KERNEL_TRACEPOINT: @@ -1444,6 +1449,7 @@ static int process_client_msg(struct command_ctx *cmd_ctx) strncpy(kctx.u.perf_counter.name, cmd_ctx->lsm->u.context.ctx.u.perf_counter.name, LTTNG_SYMBOL_NAME_LEN); + kctx.u.perf_counter.name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0'; /* Add kernel context to kernel tracer. See context.c */ ret = add_kernel_context(cmd_ctx->session->kernel_session, &kctx, diff --git a/ltt-sessiond/trace.c b/ltt-sessiond/trace.c index c090f2106..94d3c4946 100644 --- a/ltt-sessiond/trace.c +++ b/ltt-sessiond/trace.c @@ -183,6 +183,7 @@ struct ltt_kernel_event *trace_create_kernel_event(struct lttng_event *ev) attr->u.kprobe.offset = ev->attr.probe.offset; strncpy(attr->u.kprobe.symbol_name, ev->attr.probe.symbol_name, LTTNG_SYM_NAME_LEN); + attr->u.kprobe.symbol_name[LTTNG_SYM_NAME_LEN - 1] = '\0'; break; case LTTNG_EVENT_FUNCTION: attr->instrumentation = LTTNG_KERNEL_KRETPROBE; @@ -191,11 +192,13 @@ struct ltt_kernel_event *trace_create_kernel_event(struct lttng_event *ev) attr->u.kretprobe.offset = ev->attr.probe.offset; strncpy(attr->u.kretprobe.symbol_name, ev->attr.probe.symbol_name, LTTNG_SYM_NAME_LEN); + attr->u.kretprobe.symbol_name[LTTNG_SYM_NAME_LEN - 1] = '\0'; break; case LTTNG_EVENT_FUNCTION_ENTRY: attr->instrumentation = LTTNG_KERNEL_FUNCTION; strncpy(attr->u.ftrace.symbol_name, ev->attr.ftrace.symbol_name, LTTNG_SYM_NAME_LEN); + attr->u.ftrace.symbol_name[LTTNG_SYM_NAME_LEN - 1] = '\0'; break; case LTTNG_EVENT_TRACEPOINT: attr->instrumentation = LTTNG_KERNEL_TRACEPOINT; @@ -207,6 +210,7 @@ struct ltt_kernel_event *trace_create_kernel_event(struct lttng_event *ev) /* Copy event name */ strncpy(attr->name, ev->name, LTTNG_SYM_NAME_LEN); + attr->name[LTTNG_SYM_NAME_LEN - 1] = '\0'; /* Setting up a kernel event */ lke->fd = 0; diff --git a/ltt-sessiond/ust-ctl.c b/ltt-sessiond/ust-ctl.c index abee38817..d9d4af51a 100644 --- a/ltt-sessiond/ust-ctl.c +++ b/ltt-sessiond/ust-ctl.c @@ -89,7 +89,8 @@ void get_traces_per_session(struct ltt_session *session, struct lttng_trace *tra if (session->kern_session_count > 0) { trace.type = KERNEL; - strncpy(trace.name, "kernel", 6); + strncpy(trace.name, "kernel", sizeof(trace.name)); + trace.name[sizeof(trace.name) - 1] = '\0'; memcpy(&traces[i], &trace, sizeof(trace)); } } diff --git a/lttng/commands/disable_events.c b/lttng/commands/disable_events.c index 9d73ff580..60e77bdcf 100644 --- a/lttng/commands/disable_events.c +++ b/lttng/commands/disable_events.c @@ -125,6 +125,7 @@ static int disable_events(void) /* Copy name and type of the event */ strncpy(ev.name, event_name, LTTNG_SYMBOL_NAME_LEN); + ev.name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0'; ret = lttng_disable_event(&dom, event_name, channel_name); if (ret < 0) { MSG("Unable to disable event %s for channel %s", diff --git a/lttng/commands/enable_events.c b/lttng/commands/enable_events.c index 0f376ad09..5b844e493 100644 --- a/lttng/commands/enable_events.c +++ b/lttng/commands/enable_events.c @@ -125,6 +125,7 @@ static int parse_probe_opts(struct lttng_event *ev, char *opt) ret = sscanf(opt, "%[^'+']+%s", name, s_hex); if (ret == 2) { strncpy(ev->attr.probe.symbol_name, name, LTTNG_SYMBOL_NAME_LEN); + ev->attr.probe.symbol_name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0'; DBG("probe symbol %s", ev->attr.probe.symbol_name); if (strlen(s_hex) == 0) { ERR("Invalid probe offset %s", s_hex); @@ -142,6 +143,7 @@ static int parse_probe_opts(struct lttng_event *ev, char *opt) ret = sscanf(opt, "%s", name); if (ret == 1) { strncpy(ev->attr.probe.symbol_name, name, LTTNG_SYMBOL_NAME_LEN); + ev->attr.probe.symbol_name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0'; DBG("probe symbol %s", ev->attr.probe.symbol_name); ev->attr.probe.offset = 0; DBG("probe offset %" PRIu64, ev->attr.probe.offset); @@ -230,6 +232,7 @@ static int enable_events(void) event_name, channel_name); /* Copy name and type of the event */ strncpy(ev.name, event_name, LTTNG_SYMBOL_NAME_LEN); + ev.name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0'; ev.type = opt_event_type; switch (opt_event_type) { @@ -255,6 +258,7 @@ static int enable_events(void) strncpy(ev.attr.ftrace.symbol_name, opt_function_entry_symbol, LTTNG_SYMBOL_NAME_LEN); + ev.attr.ftrace.symbol_name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0'; break; default: ret = CMD_NOT_IMPLEMENTED; -- 2.34.1