[PATCH lttng-tools 01/24] Implement lttng_strncpy safe string copy

[PATCH lttng-tools 01/24] Implement lttng_strncpy safe string copy

[Mathieu Desnoyers]

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/common/macros.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/src/common/macros.h b/src/common/macros.h
index 308d3d5..6147eff 100644
--- a/src/common/macros.h
+++ b/src/common/macros.h
@@ -20,6 +20,7 @@
 #define _MACROS_H
 
 #include <stdlib.h>
+#include <string.h>
 
 /*
  * Takes a pointer x and transform it so we can use it to access members
@@ -76,4 +77,25 @@ void *zmalloc(size_t len)
 #define LTTNG_HIDDEN __attribute__((visibility("hidden")))
 #endif
 
+/*
+ * lttng_strncpy returns 0 on success, or nonzero on failure.
+ * It checks that the @src string fits into @dest_len before performing
+ * the copy. On failure, no copy has been performed.
+ */
+static inline
+int lttng_strncpy(char *dest, const char *src, size_t dest_len)
+{
+	if (strlen(src) >= dest_len) {
+		return -1;
+	}
+	strncpy(dest, src, dest_len);
+	/*
+	 * Be extra careful and put final \0 at the end after strncpy(),
+	 * even though we checked the length before. This makes Coverity
+	 * happy.
+	 */
+	dest[dest_len - 1] = '\0';
+	return 0;
+}
+
 #endif /* _MACROS_H */
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 02/24] Fix: illegal memory access in _cmd_enable_event

[Mathieu Desnoyers]

Found by Coverity:

CID 1321742 (#1 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING)21. buffer_size_warning: Calling strncpy with a
maximum size argument of 256 bytes on destination array attr->name of
size 256 bytes might leave the destination string unterminated.

CID 1321742 (#2 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING)22. buffer_size_warning: Calling strncpy with a
maximum size argument of 256 bytes on destination array attr->name of
size 256 bytes might leave the destination string unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/bin/lttng-sessiond/cmd.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
index 16d8ba2..b72b091 100644
--- a/src/bin/lttng-sessiond/cmd.c
+++ b/src/bin/lttng-sessiond/cmd.c
@@ -1853,7 +1853,12 @@ static int _cmd_enable_event(struct ltt_session *session,
 				ret = LTTNG_ERR_FATAL;
 				goto error;
 			}
-			strncpy(attr->name, channel_name, sizeof(attr->name));
+			if (lttng_strncpy(attr->name, channel_name,
+					sizeof(attr->name))) {
+				ret = LTTNG_ERR_INVALID;
+				free(attr);
+				goto error;
+			}
 
 			ret = cmd_enable_channel(session, domain, attr, wpipe);
 			if (ret != LTTNG_OK) {
@@ -1990,7 +1995,12 @@ static int _cmd_enable_event(struct ltt_session *session,
 				ret = LTTNG_ERR_FATAL;
 				goto error;
 			}
-			strncpy(attr->name, channel_name, sizeof(attr->name));
+			if (lttng_strncpy(attr->name, channel_name,
+					sizeof(attr->name))) {
+				ret = LTTNG_ERR_INVALID;
+				free(attr);
+				goto error;
+			}
 
 			ret = cmd_enable_channel(session, domain, attr, wpipe);
 			if (ret != LTTNG_OK) {
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 03/24] Fix: illegal memory access in init_ust_event_from_agent_event

[Mathieu Desnoyers]

Found by Coverity:
CID 1321741 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING)1. buffer_size_warning: Calling strncpy with a
maximum size argument of 256 bytes on destination array
ust_event->attr.name of size 256 bytes might leave the destination
string unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/bin/lttng-sessiond/save.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/bin/lttng-sessiond/save.c b/src/bin/lttng-sessiond/save.c
index 489446d..9ac7712 100644
--- a/src/bin/lttng-sessiond/save.c
+++ b/src/bin/lttng-sessiond/save.c
@@ -708,7 +708,11 @@ int init_ust_event_from_agent_event(struct ltt_ust_event *ust_event,
 
 	ust_event->enabled = agent_event->enabled;
 	ust_event->attr.instrumentation = LTTNG_UST_TRACEPOINT;
-	strncpy(ust_event->attr.name, agent_event->name, LTTNG_SYMBOL_NAME_LEN);
+	if (lttng_strncpy(ust_event->attr.name, agent_event->name,
+			LTTNG_SYMBOL_NAME_LEN)) {
+		ret = -1;
+		goto end;
+	}
 	switch (agent_event->loglevel_type) {
 	case LTTNG_EVENT_LOGLEVEL_ALL:
 		ust_loglevel_type = LTTNG_UST_LOGLEVEL_ALL;
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 04/24] Fix: illegal memory access in add_uri_to_consumer

[Mathieu Desnoyers]

Found by Coverity:

CID 1243038 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING)15. buffer_size_warning: Calling strncpy with a
maximum size argument of 4096 bytes on destination array
consumer->dst.trace_path of size 4096 bytes might leave the destination
string unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/bin/lttng-sessiond/cmd.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
index b72b091..5afa1a3 100644
--- a/src/bin/lttng-sessiond/cmd.c
+++ b/src/bin/lttng-sessiond/cmd.c
@@ -758,12 +758,15 @@ static int add_uri_to_consumer(struct consumer_output *consumer,
 		DBG2("Setting trace directory path from URI to %s", uri->dst.path);
 		memset(consumer->dst.trace_path, 0,
 				sizeof(consumer->dst.trace_path));
-		strncpy(consumer->dst.trace_path, uri->dst.path,
-				sizeof(consumer->dst.trace_path));
+		/* Explicit length checks for strcpy and strcat. */
+		if (strlen(uri->dst.path) + strlen(default_trace_dir)
+				>= sizeof(consumer->dst.trace_path)) {
+			ret = LTTNG_ERR_FATAL;
+			goto error;
+		}
+		strcpy(consumer->dst.trace_path, uri->dst.path);
 		/* Append default trace dir */
-		strncat(consumer->dst.trace_path, default_trace_dir,
-				sizeof(consumer->dst.trace_path) -
-				strlen(consumer->dst.trace_path) - 1);
+		strcat(consumer->dst.trace_path, default_trace_dir);
 		/* Flag consumer as local. */
 		consumer->type = CONSUMER_DST_LOCAL;
 		break;
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 05/24] Fix: illegal memory access in enable_event

[Mathieu Desnoyers]

Found by Coverity:
CID 1243033 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING)16. buffer_size_warning: Calling strncpy with a
maximum size argument of 256 bytes on destination array msg.name of size
256 bytes might leave the destination string unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/bin/lttng-sessiond/agent.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/bin/lttng-sessiond/agent.c b/src/bin/lttng-sessiond/agent.c
index ced0f85..f79ac00 100644
--- a/src/bin/lttng-sessiond/agent.c
+++ b/src/bin/lttng-sessiond/agent.c
@@ -408,17 +408,20 @@ static int enable_event(struct agent_app *app, struct agent_event *event)
 	}
 	data_size = sizeof(msg) + filter_expression_length;
 
-	ret = send_header(app->sock, data_size, AGENT_CMD_ENABLE, 0);
-	if (ret < 0) {
-		goto error_io;
-	}
-
 	memset(&msg, 0, sizeof(msg));
 	msg.loglevel_value = htobe32(event->loglevel_value);
 	msg.loglevel_type = htobe32(event->loglevel_type);
-	strncpy(msg.name, event->name, sizeof(msg.name));
+	if (lttng_strncpy(msg.name, event->name, sizeof(msg.name))) {
+		ret = LTTNG_ERR_INVALID;
+		goto error;
+	}
 	msg.filter_expression_length = htobe32(filter_expression_length);
 
+	ret = send_header(app->sock, data_size, AGENT_CMD_ENABLE, 0);
+	if (ret < 0) {
+		goto error_io;
+	}
+
 	bytes_to_send = zmalloc(data_size);
 	if (!bytes_to_send) {
 		ret = LTTNG_ERR_NOMEM;
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 06/24] Fix: illegal memory access in disable_event

[Mathieu Desnoyers]

Found by Coverity:
CID 1243016 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING)14. buffer_size_warning: Calling strncpy with a
maximum size argument of 256 bytes on destination array msg.name of size
256 bytes might leave the destination string unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/bin/lttng-sessiond/agent.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/bin/lttng-sessiond/agent.c b/src/bin/lttng-sessiond/agent.c
index f79ac00..6841d41 100644
--- a/src/bin/lttng-sessiond/agent.c
+++ b/src/bin/lttng-sessiond/agent.c
@@ -594,14 +594,17 @@ static int disable_event(struct agent_app *app, struct agent_event *event)
 			app->pid, app->sock->fd);
 
 	data_size = sizeof(msg);
+	memset(&msg, 0, sizeof(msg));
+	if (lttng_strncpy(msg.name, event->name, sizeof(msg.name))) {
+		ret = LTTNG_ERR_INVALID;
+		goto error;
+	}
 
 	ret = send_header(app->sock, data_size, AGENT_CMD_DISABLE, 0);
 	if (ret < 0) {
 		goto error_io;
 	}
 
-	memset(&msg, 0, sizeof(msg));
-	strncpy(msg.name, event->name, sizeof(msg.name));
 	ret = send_payload(app->sock, &msg, sizeof(msg));
 	if (ret < 0) {
 		goto error_io;
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 08/24] Fix: illegal memory access in cmd_snapshot_list_outputs

[Mathieu Desnoyers]

Found by Coverity:

CID 1243031 (#1 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING)22. buffer_size_warning: Calling strncpy with a
maximum size argument of 4096 bytes on destination array (list +
idx).ctrl_url of size 4096 bytes might leave the destination string
unterminated.

CID 1243031 (#2 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING)26. buffer_size_warning: Calling strncpy with a
maximum size argument of 255 bytes on destination array (list +
idx).name of size 255 bytes might leave the destination string
unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/bin/lttng-sessiond/cmd.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
index 5afa1a3..23e7210 100644
--- a/src/bin/lttng-sessiond/cmd.c
+++ b/src/bin/lttng-sessiond/cmd.c
@@ -3362,10 +3362,18 @@ ssize_t cmd_snapshot_list_outputs(struct ltt_session *session,
 		assert(output->consumer);
 		list[idx].id = output->id;
 		list[idx].max_size = output->max_size;
-		strncpy(list[idx].name, output->name, sizeof(list[idx].name));
+		if (lttng_strncpy(list[idx].name, output->name,
+				sizeof(list[idx].name))) {
+			ret = -LTTNG_ERR_INVALID;
+			goto error;
+		}
 		if (output->consumer->type == CONSUMER_DST_LOCAL) {
-			strncpy(list[idx].ctrl_url, output->consumer->dst.trace_path,
-					sizeof(list[idx].ctrl_url));
+			if (lttng_strncpy(list[idx].ctrl_url,
+					output->consumer->dst.trace_path,
+					sizeof(list[idx].ctrl_url))) {
+				ret = -LTTNG_ERR_INVALID;
+				goto error;
+			}
 		} else {
 			/* Control URI. */
 			ret = uri_to_str_url(&output->consumer->dst.net.control,
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 09/24] Fix: illegal memory access in consumer_set_network_uri

[Mathieu Desnoyers]

Found by Coverity:
CID 1243029 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING)31. buffer_size_warning: Calling strncpy with a
maximum size argument of 4096 bytes on destination array obj->subdir of
size 4096 bytes might leave the destination string unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/bin/lttng-sessiond/consumer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/bin/lttng-sessiond/consumer.c b/src/bin/lttng-sessiond/consumer.c
index bd019dd..a8c5fb8 100644
--- a/src/bin/lttng-sessiond/consumer.c
+++ b/src/bin/lttng-sessiond/consumer.c
@@ -715,7 +715,10 @@ int consumer_set_network_uri(struct consumer_output *obj,
 			goto error;
 		}
 
-		strncpy(obj->subdir, tmp_path, sizeof(obj->subdir));
+		if (lttng_strncpy(obj->subdir, tmp_path, sizeof(obj->subdir))) {
+			ret = -LTTNG_ERR_INVALID;
+			goto error;
+		}
 		DBG3("Consumer set network uri subdir path %s", tmp_path);
 	}
 
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 10/24] Fix: illegal memory access in output_init

[Mathieu Desnoyers]

Found by Coverity:

CID 1243028 (#1 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING)5. buffer_size_warning: Calling strncpy with a
maximum size argument of 255 bytes on destination array output->name of
size 255 bytes might leave the destination string unterminated.

CID 1243028 (#2 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING)10. buffer_size_warning: Calling strncpy with a
maximum size argument of 4096 bytes on destination array
output->consumer->dst.trace_path of size 4096 bytes might leave the
destination string unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/bin/lttng-sessiond/snapshot.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/bin/lttng-sessiond/snapshot.c b/src/bin/lttng-sessiond/snapshot.c
index 3de468a..d2016a1 100644
--- a/src/bin/lttng-sessiond/snapshot.c
+++ b/src/bin/lttng-sessiond/snapshot.c
@@ -62,7 +62,10 @@ static int output_init(uint64_t max_size, const char *name,
 	lttng_ht_node_init_ulong(&output->node, (unsigned long) output->id);
 
 	if (name && name[0] != '\0') {
-		strncpy(output->name, name, sizeof(output->name));
+		if (lttng_strncpy(output->name, name, sizeof(output->name))) {
+			ret = -LTTNG_ERR_INVALID;
+			goto error;
+		}
 	} else {
 		/* Set default name. */
 		ret = snprintf(output->name, sizeof(output->name), "%s-%" PRIu32,
@@ -93,8 +96,12 @@ static int output_init(uint64_t max_size, const char *name,
 	if (uris[0].dtype == LTTNG_DST_PATH) {
 		memset(output->consumer->dst.trace_path, 0,
 				sizeof(output->consumer->dst.trace_path));
-		strncpy(output->consumer->dst.trace_path, uris[0].dst.path,
-				sizeof(output->consumer->dst.trace_path));
+		if (lttng_strncpy(output->consumer->dst.trace_path,
+				uris[0].dst.path,
+				sizeof(output->consumer->dst.trace_path))) {
+			ret = -LTTNG_ERR_INVALID;
+			goto error;
+		}
 		output->consumer->type = CONSUMER_DST_LOCAL;
 		ret = 0;
 		goto end;
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 12/24] Fix: illegal memory access in list_lttng_channels

[Mathieu Desnoyers]

Found by Coverity:
CID 1243018 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING)11. buffer_size_warning: Calling strncpy with a
maximum size argument of 256 bytes on destination array (channels +
i).name of size 256 bytes might leave the destination string
unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/bin/lttng-sessiond/cmd.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
index 7f47818..e66c99f 100644
--- a/src/bin/lttng-sessiond/cmd.c
+++ b/src/bin/lttng-sessiond/cmd.c
@@ -277,7 +277,11 @@ static void list_lttng_channels(enum lttng_domain_type domain,
 				&iter.iter, uchan, node.node) {
 			uint64_t discarded_events = 0, lost_packets = 0;
 
-			strncpy(channels[i].name, uchan->name, LTTNG_SYMBOL_NAME_LEN);
+			if (lttng_strncpy(channels[i].name, uchan->name,
+					LTTNG_SYMBOL_NAME_LEN)) {
+				ret = -1;
+				break;
+			}
 			channels[i].attr.overwrite = uchan->attr.overwrite;
 			channels[i].attr.subbuf_size = uchan->attr.subbuf_size;
 			channels[i].attr.num_subbuf = uchan->attr.num_subbuf;
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 14/24] Fix: illegal memory access in syscall_init_table

[Mathieu Desnoyers]

Found by Coverity:
CID 1243021 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING)25. buffer_size_warning: Calling strncpy with a
maximum size argument of 255 bytes on destination array (syscall_table +
index).name of size 255 bytes might leave the destination string
unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/bin/lttng-sessiond/syscall.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/bin/lttng-sessiond/syscall.c b/src/bin/lttng-sessiond/syscall.c
index 7ae6682..02fec15 100644
--- a/src/bin/lttng-sessiond/syscall.c
+++ b/src/bin/lttng-sessiond/syscall.c
@@ -108,8 +108,11 @@ int syscall_init_table(void)
 		}
 		syscall_table[index].index = index;
 		syscall_table[index].bitness = bitness;
-		strncpy(syscall_table[index].name, name,
-				sizeof(syscall_table[index].name));
+		if (lttng_strncpy(syscall_table[index].name, name,
+				sizeof(syscall_table[index].name))) {
+			ret = -EINVAL;
+			goto error;
+		}
 		/*
 		DBG("Syscall name '%s' at index %" PRIu32 " of bitness %u",
 				syscall_table[index].name,
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 15/24] Fix: illegal memory access in consumer_set_subdir

[Mathieu Desnoyers]

Found by Coverity:
CID 1243015 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING)8. buffer_size_warning: Calling strncpy with a
maximum size argument of 4096 bytes on destination array
consumer->subdir of size 4096 bytes might leave the destination string
unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/bin/lttng-sessiond/consumer.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/bin/lttng-sessiond/consumer.c b/src/bin/lttng-sessiond/consumer.c
index a8c5fb8..e9e430e 100644
--- a/src/bin/lttng-sessiond/consumer.c
+++ b/src/bin/lttng-sessiond/consumer.c
@@ -1089,7 +1089,11 @@ int consumer_set_subdir(struct consumer_output *consumer,
 		goto error;
 	}
 
-	strncpy(consumer->subdir, tmp_path, sizeof(consumer->subdir));
+	if (lttng_strncpy(consumer->subdir, tmp_path,
+			sizeof(consumer->subdir))) {
+		ret = -1;
+		goto error;
+	}
 	DBG2("Consumer subdir set to %s", consumer->subdir);
 
 error:
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 17/24] Fix: illegal memory access in relayd_create_session_2_4

[Mathieu Desnoyers]

Found by Coverity:
CID 1243024 (#1 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING)2. buffer_size_warning: Calling strncpy with a
maximum size argument of 255 bytes on destination array msg.session_name
of size 255 bytes might leave the destination string unterminated.

CID 1243024 (#2 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING)3. buffer_size_warning: Calling strncpy with a
maximum size argument of 64 bytes on destination array msg.hostname of
size 64 bytes might leave the destination string unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/common/relayd/relayd.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/common/relayd/relayd.c b/src/common/relayd/relayd.c
index acf6c38..9e95255 100644
--- a/src/common/relayd/relayd.c
+++ b/src/common/relayd/relayd.c
@@ -129,16 +129,15 @@ static int relayd_create_session_2_4(struct lttcomm_relayd_sock *rsock,
 	int ret;
 	struct lttcomm_relayd_create_session_2_4 msg;
 
-	if (strlen(session_name) >= sizeof(msg.session_name)) {
+	if (lttng_strncpy(msg.session_name, session_name,
+			sizeof(msg.session_name))) {
 		ret = -1;
 		goto error;
 	}
-	strncpy(msg.session_name, session_name, sizeof(msg.session_name));
-	if (strlen(hostname) >= sizeof(msg.hostname)) {
+	if (lttng_strncpy(msg.hostname, hostname, sizeof(msg.hostname))) {
 		ret = -1;
 		goto error;
 	}
-	strncpy(msg.hostname, hostname, sizeof(msg.hostname));
 	msg.live_timer = htobe32(session_live_timer);
 	msg.snapshot = htobe32(snapshot);
 
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 18/24] Fix: illegal memory access in relayd_add_stream

[Mathieu Desnoyers]

Found by Coverity:

CID 1243017 (#1 of 4): Buffer not null terminated
(BUFFER_SIZE_WARNING)14. buffer_size_warning: Calling strncpy with a
maximum size argument of 264 bytes on destination array msg.channel_name
of size 264 bytes might leave the destination string unterminated.

ID 1243017 (#2 of 4): Buffer not null terminated
(BUFFER_SIZE_WARNING)14. buffer_size_warning: Calling strncpy with a
maximum size argument of 264 bytes on destination array
msg_2_2.channel_name of size 264 bytes might leave the destination
string unterminated.

CID 1243017 (#3 of 4): Buffer not null terminated
(BUFFER_SIZE_WARNING)15. buffer_size_warning: Calling strncpy with a
maximum size argument of 4096 bytes on destination array msg.pathname of
size 4096 bytes might leave the destination string unterminated.

CID 1243017 (#4 of 4): Buffer not null terminated
(BUFFER_SIZE_WARNING)15. buffer_size_warning: Calling strncpy with a
maximum size argument of 4096 bytes on destination array
msg_2_2.pathname of size 4096 bytes might leave the destination string
unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/common/relayd/relayd.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/common/relayd/relayd.c b/src/common/relayd/relayd.c
index 9e95255..7f0ea74 100644
--- a/src/common/relayd/relayd.c
+++ b/src/common/relayd/relayd.c
@@ -254,16 +254,16 @@ int relayd_add_stream(struct lttcomm_relayd_sock *rsock, const char *channel_nam
 	/* Compat with relayd 2.1 */
 	if (rsock->minor == 1) {
 		memset(&msg, 0, sizeof(msg));
-		if (strlen(channel_name) >= sizeof(msg.channel_name)) {
+		if (lttng_strncpy(msg.channel_name, channel_name,
+				sizeof(msg.channel_name))) {
 			ret = -1;
 			goto error;
 		}
-		strncpy(msg.channel_name, channel_name, sizeof(msg.channel_name));
-		if (strlen(pathname) >= sizeof(msg.pathname)) {
+		if (lttng_strncpy(msg.pathname, pathname,
+				sizeof(msg.pathname))) {
 			ret = -1;
 			goto error;
 		}
-		strncpy(msg.pathname, pathname, sizeof(msg.pathname));
 
 		/* Send command */
 		ret = send_command(rsock, RELAYD_ADD_STREAM, (void *) &msg, sizeof(msg), 0);
@@ -273,16 +273,16 @@ int relayd_add_stream(struct lttcomm_relayd_sock *rsock, const char *channel_nam
 	} else {
 		memset(&msg_2_2, 0, sizeof(msg_2_2));
 		/* Compat with relayd 2.2+ */
-		if (strlen(channel_name) >= sizeof(msg_2_2.channel_name)) {
+		if (lttng_strncpy(msg_2_2.channel_name, channel_name,
+				sizeof(msg_2_2.channel_name))) {
 			ret = -1;
 			goto error;
 		}
-		strncpy(msg_2_2.channel_name, channel_name, sizeof(msg_2_2.channel_name));
-		if (strlen(pathname) >= sizeof(msg_2_2.pathname)) {
+		if (lttng_strncpy(msg_2_2.pathname, pathname,
+				sizeof(msg_2_2.pathname))) {
 			ret = -1;
 			goto error;
 		}
-		strncpy(msg_2_2.pathname, pathname, sizeof(msg_2_2.pathname));
 		msg_2_2.tracefile_size = htobe64(tracefile_size);
 		msg_2_2.tracefile_count = htobe64(tracefile_count);
 
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 20/24] Fix: illegal memory access in send_viewer_streams

[Mathieu Desnoyers]

Found by Coverity:

CID 1243037 (#1 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING)18. buffer_size_warning: Calling strncpy with a
maximum size argument of 4096 bytes on destination array
send_stream.path_name of size 4096 bytes might leave the destination
string unterminated.

CID 1243037 (#2 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING)18. buffer_size_warning: Calling strncpy with a
maximum size argument of 255 bytes on destination array
send_stream.channel_name of size 255 bytes might leave the destination
string unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 src/bin/lttng-relayd/live.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/bin/lttng-relayd/live.c b/src/bin/lttng-relayd/live.c
index f87e4ba..82bd2bd 100644
--- a/src/bin/lttng-relayd/live.c
+++ b/src/bin/lttng-relayd/live.c
@@ -230,10 +230,21 @@ ssize_t send_viewer_streams(struct lttcomm_sock *sock,
 		send_stream.ctf_trace_id = htobe64(ctf_trace->id);
 		send_stream.metadata_flag = htobe32(
 				vstream->stream->is_metadata);
-		strncpy(send_stream.path_name, vstream->path_name,
-				sizeof(send_stream.path_name));
-		strncpy(send_stream.channel_name, vstream->channel_name,
-				sizeof(send_stream.channel_name));
+		if (lttng_strncpy(send_stream.path_name, vstream->path_name,
+				sizeof(send_stream.path_name))) {
+			pthread_mutex_unlock(&vstream->stream->lock);
+			viewer_stream_put(vstream);
+			ret = -1;	/* Error. */
+			goto end_unlock;
+		}
+		if (lttng_strncpy(send_stream.channel_name,
+				vstream->channel_name,
+				sizeof(send_stream.channel_name))) {
+			pthread_mutex_unlock(&vstream->stream->lock);
+			viewer_stream_put(vstream);
+			ret = -1;	/* Error. */
+			goto end_unlock;
+		}
 
 		DBG("Sending stream %" PRIu64 " to viewer",
 				vstream->stream->stream_handle);
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[PATCH lttng-tools 22/24] Fix: illegal memory access in test_create_kernel_event

[Mathieu Desnoyers]

Found by Coverity:
CID 1243030 (#1 of 1): Buffer not null terminated (BUFFER_SIZE_WARNING)1.
buffer_size_warning: Calling strncpy with a maximum size argument of 256
bytes on destination array ev.name of size 256 bytes might leave the
destination string unterminated.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 tests/unit/test_kernel_data.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/unit/test_kernel_data.c b/tests/unit/test_kernel_data.c
index f1e1b38..fde46f7 100644
--- a/tests/unit/test_kernel_data.c
+++ b/tests/unit/test_kernel_data.c
@@ -32,7 +32,7 @@
 #define RANDOM_STRING_LEN	11
 
 /* Number of TAP tests in this file */
-#define NUM_TESTS 10
+#define NUM_TESTS 11
 
 /* For error.h */
 int lttng_opt_quiet = 1;
@@ -134,7 +134,9 @@ static void test_create_kernel_event(void)
 	struct lttng_event ev;
 
 	memset(&ev, 0, sizeof(ev));
-	strncpy(ev.name, get_random_string(), LTTNG_KERNEL_SYM_NAME_LEN);
+	ok(lttng_strncpy(ev.name, get_random_string(),
+			LTTNG_KERNEL_SYM_NAME_LEN),
+		"Validate string length");
 	ev.type = LTTNG_EVENT_TRACEPOINT;
 	ev.loglevel_type = LTTNG_EVENT_LOGLEVEL_ALL;
 
-- 
2.1.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [PATCH lttng-tools 00/24] Fix illegal memory accesses

[Jérémie Galarneau]

All 24 patches have been merged in master (some with minor changes,
see replies to indvidual patches). I'll merge them in 2.8 and see what
make sense to backport to 2.7 and 2.6.

Thanks!
Jérémie

On Mon, May 16, 2016 at 9:42 PM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> Coverity found illegal memory accesses around the use of strncpy().
> Implement a lttng_strncpy() which performs sanity check with strlen()
> first, and fails if the input string is too long to fit in the
> destination length.
>
> Mathieu Desnoyers (24):
>   Implement lttng_strncpy safe string copy
>   Fix: illegal memory access in _cmd_enable_event
>   Fix: illegal memory access in init_ust_event_from_agent_event
>   Fix: illegal memory access in add_uri_to_consumer
>   Fix: illegal memory access in enable_event
>   Fix: illegal memory access in disable_event
>   Fix: illegal memory access in list_events
>   Fix: illegal memory access in cmd_snapshot_list_outputs
>   Fix: illegal memory access in consumer_set_network_uri
>   Fix: illegal memory access in output_init
>   Fix: illegal memory access in cmd_snapshot_record
>   Fix: illegal memory access in list_lttng_channels
>   Fix: illegal memory access in write_pidfile
>   Fix: illegal memory access in syscall_init_table
>   Fix: illegal memory access in consumer_set_subdir
>   Fix: illegal memory access in session_create
>   Fix: illegal memory access in relayd_create_session_2_4
>   Fix: illegal memory access in relayd_add_stream
>   Fix: illegal memory access in viewer_list_sessions
>   Fix: illegal memory access in send_viewer_streams
>   Fix: illegal memory access in test_create_ust_channel
>   Fix: illegal memory access in test_create_kernel_event
>   Fix: illegal memory access in test_create_ust_event
>   Fix: illegal memory access in test_create_ust_event_exclusion
>
>  src/bin/lttng-relayd/live.c       | 36 +++++++++++++++++++------
>  src/bin/lttng-relayd/session.c    | 19 ++++++++------
>  src/bin/lttng-sessiond/agent.c    | 29 ++++++++++++++-------
>  src/bin/lttng-sessiond/cmd.c      | 55 ++++++++++++++++++++++++++++++---------
>  src/bin/lttng-sessiond/consumer.c | 11 ++++++--
>  src/bin/lttng-sessiond/main.c     |  5 +++-
>  src/bin/lttng-sessiond/save.c     |  6 ++++-
>  src/bin/lttng-sessiond/snapshot.c | 13 ++++++---
>  src/bin/lttng-sessiond/syscall.c  |  7 +++--
>  src/common/macros.h               | 12 +++++++++
>  src/common/relayd/relayd.c        | 23 ++++++++--------
>  tests/unit/test_kernel_data.c     |  6 +++--
>  tests/unit/test_ust_data.c        | 13 +++++----
>  13 files changed, 168 insertions(+), 67 deletions(-)
>
> --
> 2.1.4
>



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [PATCH lttng-tools 01/24] Implement lttng_strncpy safe string copy

[Jérémie Galarneau]

On Mon, May 16, 2016 at 9:42 PM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> ---
>  src/common/macros.h | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/src/common/macros.h b/src/common/macros.h
> index 308d3d5..6147eff 100644
> --- a/src/common/macros.h
> +++ b/src/common/macros.h
> @@ -20,6 +20,7 @@
>  #define _MACROS_H
>
>  #include <stdlib.h>
> +#include <string.h>
>
>  /*
>   * Takes a pointer x and transform it so we can use it to access members
> @@ -76,4 +77,25 @@ void *zmalloc(size_t len)
>  #define LTTNG_HIDDEN __attribute__((visibility("hidden")))
>  #endif
>
> +/*
> + * lttng_strncpy returns 0 on success, or nonzero on failure.
> + * It checks that the @src string fits into @dest_len before performing
> + * the copy. On failure, no copy has been performed.

Added comment to mention that dest_len includes the NULL delimiter.

> + */
> +static inline
> +int lttng_strncpy(char *dest, const char *src, size_t dest_len)
> +{
> +       if (strlen(src) >= dest_len) {

Switching strlen() to strnlen() to protect against cases such as in
"Fix: illegal memory access in init_ust_event_from_agent_event" where
the source may not be NULL-terminated (even though it should be). I
don't want to change the behavior this close to the release.

A lot of code currently assumes names/identifiers to be bounded by
LTTNG_SYMBOL_NAME_LEN which could cause the strlen() run
to overrun here.

Jérémie

> +               return -1;
> +       }
> +       strncpy(dest, src, dest_len);
> +       /*
> +        * Be extra careful and put final \0 at the end after strncpy(),
> +        * even though we checked the length before. This makes Coverity
> +        * happy.
> +        */
> +       dest[dest_len - 1] = '\0';
> +       return 0;
> +}
> +
>  #endif /* _MACROS_H */
> --
> 2.1.4
>



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [PATCH lttng-tools 12/24] Fix: illegal memory access in list_lttng_channels

[Jérémie Galarneau]

On Mon, May 16, 2016 at 9:42 PM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> Found by Coverity:
> CID 1243018 (#1 of 1): Buffer not null terminated
> (BUFFER_SIZE_WARNING)11. buffer_size_warning: Calling strncpy with a
> maximum size argument of 256 bytes on destination array (channels +
> i).name of size 256 bytes might leave the destination string
> unterminated.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> ---
>  src/bin/lttng-sessiond/cmd.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
> index 7f47818..e66c99f 100644
> --- a/src/bin/lttng-sessiond/cmd.c
> +++ b/src/bin/lttng-sessiond/cmd.c
> @@ -277,7 +277,11 @@ static void list_lttng_channels(enum lttng_domain_type domain,
>                                 &iter.iter, uchan, node.node) {
>                         uint64_t discarded_events = 0, lost_packets = 0;
>
> -                       strncpy(channels[i].name, uchan->name, LTTNG_SYMBOL_NAME_LEN);
> +                       if (lttng_strncpy(channels[i].name, uchan->name,
> +                                       LTTNG_SYMBOL_NAME_LEN)) {
> +                               ret = -1;

Removed ret = -1 since ret is never used (the function has no return value).

Jérémie

> +                               break;
> +                       }
>                         channels[i].attr.overwrite = uchan->attr.overwrite;
>                         channels[i].attr.subbuf_size = uchan->attr.subbuf_size;
>                         channels[i].attr.num_subbuf = uchan->attr.num_subbuf;
> --
> 2.1.4
>



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [PATCH lttng-tools 14/24] Fix: illegal memory access in syscall_init_table

[Jérémie Galarneau]

On Mon, May 16, 2016 at 9:42 PM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> Found by Coverity:
> CID 1243021 (#1 of 1): Buffer not null terminated
> (BUFFER_SIZE_WARNING)25. buffer_size_warning: Calling strncpy with a
> maximum size argument of 255 bytes on destination array (syscall_table +
> index).name of size 255 bytes might leave the destination string
> unterminated.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> ---
>  src/bin/lttng-sessiond/syscall.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/syscall.c b/src/bin/lttng-sessiond/syscall.c
> index 7ae6682..02fec15 100644
> --- a/src/bin/lttng-sessiond/syscall.c
> +++ b/src/bin/lttng-sessiond/syscall.c
> @@ -108,8 +108,11 @@ int syscall_init_table(void)
>                 }
>                 syscall_table[index].index = index;
>                 syscall_table[index].bitness = bitness;
> -               strncpy(syscall_table[index].name, name,
> -                               sizeof(syscall_table[index].name));
> +               if (lttng_strncpy(syscall_table[index].name, name,
> +                               sizeof(syscall_table[index].name))) {
> +                       ret = -EINVAL;
> +                       goto error;

This will leak syscall_table. I have made the change in the merged
version (free() and set to NULL).

Jérémie

> +               }
>                 /*
>                 DBG("Syscall name '%s' at index %" PRIu32 " of bitness %u",
>                                 syscall_table[index].name,
> --
> 2.1.4
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [PATCH lttng-tools 01/24] Implement lttng_strncpy safe string copy

[Mathieu Desnoyers]

----- On May 17, 2016, at 11:55 AM, Jeremie Galarneau jeremie.galarneau@efficios.com wrote:

> On Mon, May 16, 2016 at 9:42 PM, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> ---
>>  src/common/macros.h | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/src/common/macros.h b/src/common/macros.h
>> index 308d3d5..6147eff 100644
>> --- a/src/common/macros.h
>> +++ b/src/common/macros.h
>> @@ -20,6 +20,7 @@
>>  #define _MACROS_H
>>
>>  #include <stdlib.h>
>> +#include <string.h>
>>
>>  /*
>>   * Takes a pointer x and transform it so we can use it to access members
>> @@ -76,4 +77,25 @@ void *zmalloc(size_t len)
>>  #define LTTNG_HIDDEN __attribute__((visibility("hidden")))
>>  #endif
>>
>> +/*
>> + * lttng_strncpy returns 0 on success, or nonzero on failure.
>> + * It checks that the @src string fits into @dest_len before performing
>> + * the copy. On failure, no copy has been performed.
> 
> Added comment to mention that dest_len includes the NULL delimiter.
> 
>> + */
>> +static inline
>> +int lttng_strncpy(char *dest, const char *src, size_t dest_len)
>> +{
>> +       if (strlen(src) >= dest_len) {
> 
> Switching strlen() to strnlen() to protect against cases such as in
> "Fix: illegal memory access in init_ust_event_from_agent_event" where
> the source may not be NULL-terminated (even though it should be). I
> don't want to change the behavior this close to the release.
> 
> A lot of code currently assumes names/identifiers to be bounded by
> LTTNG_SYMBOL_NAME_LEN which could cause the strlen() run
> to overrun here.

Good idea! thanks!

Mathieu

> 
> Jérémie
> 
>> +               return -1;
>> +       }
>> +       strncpy(dest, src, dest_len);
>> +       /*
>> +        * Be extra careful and put final \0 at the end after strncpy(),
>> +        * even though we checked the length before. This makes Coverity
>> +        * happy.
>> +        */
>> +       dest[dest_len - 1] = '\0';
>> +       return 0;
>> +}
>> +
>>  #endif /* _MACROS_H */
>> --
>> 2.1.4
>>
> 
> 
> 
> --
> Jérémie Galarneau
> EfficiOS Inc.
> http://www.efficios.com

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev