From 85d1db9fbef6b3e6148bbaf3235b01d446c6c07f Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Fri, 13 Sep 2019 16:48:18 -0400 Subject: [PATCH] consumer: fix: unaligned accesses to index fields MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The ctf_index structure, being part of the ABI, is explicitly packed using the LTTNG_PACKED macro. However, populating it by using pointers to its members is not acceptable as it may cause the ust and kernel tracer APIs to populate write their return values using unaligned pointers. Use automatic storage variables to fetch the various index fields and populate the index at-once using a compound literal. Signed-off-by: Jérémie Galarneau --- src/common/kernel-consumer/kernel-consumer.c | 42 ++++++++++++-------- src/common/ust-consumer/ust-consumer.c | 39 ++++++++++-------- 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/src/common/kernel-consumer/kernel-consumer.c b/src/common/kernel-consumer/kernel-consumer.c index 29a27134f..aea1b4831 100644 --- a/src/common/kernel-consumer/kernel-consumer.c +++ b/src/common/kernel-consumer/kernel-consumer.c @@ -1327,66 +1327,62 @@ end: static int get_index_values(struct ctf_packet_index *index, int infd) { int ret; + uint64_t packet_size, content_size, timestamp_begin, timestamp_end, + events_discarded, stream_id, stream_instance_id, + packet_seq_num; - ret = kernctl_get_timestamp_begin(infd, &index->timestamp_begin); + ret = kernctl_get_timestamp_begin(infd, ×tamp_begin); if (ret < 0) { PERROR("kernctl_get_timestamp_begin"); goto error; } - index->timestamp_begin = htobe64(index->timestamp_begin); - ret = kernctl_get_timestamp_end(infd, &index->timestamp_end); + ret = kernctl_get_timestamp_end(infd, ×tamp_end); if (ret < 0) { PERROR("kernctl_get_timestamp_end"); goto error; } - index->timestamp_end = htobe64(index->timestamp_end); - ret = kernctl_get_events_discarded(infd, &index->events_discarded); + ret = kernctl_get_events_discarded(infd, &events_discarded); if (ret < 0) { PERROR("kernctl_get_events_discarded"); goto error; } - index->events_discarded = htobe64(index->events_discarded); - ret = kernctl_get_content_size(infd, &index->content_size); + ret = kernctl_get_content_size(infd, &content_size); if (ret < 0) { PERROR("kernctl_get_content_size"); goto error; } - index->content_size = htobe64(index->content_size); - ret = kernctl_get_packet_size(infd, &index->packet_size); + ret = kernctl_get_packet_size(infd, &packet_size); if (ret < 0) { PERROR("kernctl_get_packet_size"); goto error; } - index->packet_size = htobe64(index->packet_size); - ret = kernctl_get_stream_id(infd, &index->stream_id); + ret = kernctl_get_stream_id(infd, &stream_id); if (ret < 0) { PERROR("kernctl_get_stream_id"); goto error; } - index->stream_id = htobe64(index->stream_id); - ret = kernctl_get_instance_id(infd, &index->stream_instance_id); + ret = kernctl_get_instance_id(infd, &stream_instance_id); if (ret < 0) { if (ret == -ENOTTY) { /* Command not implemented by lttng-modules. */ - index->stream_instance_id = -1ULL; + stream_instance_id = -1ULL; } else { PERROR("kernctl_get_instance_id"); goto error; } } - index->stream_instance_id = htobe64(index->stream_instance_id); - ret = kernctl_get_sequence_number(infd, &index->packet_seq_num); + ret = kernctl_get_sequence_number(infd, &packet_seq_num); if (ret < 0) { if (ret == -ENOTTY) { /* Command not implemented by lttng-modules. */ - index->packet_seq_num = -1ULL; + packet_seq_num = -1ULL; ret = 0; } else { PERROR("kernctl_get_sequence_number"); @@ -1395,6 +1391,18 @@ static int get_index_values(struct ctf_packet_index *index, int infd) } index->packet_seq_num = htobe64(index->packet_seq_num); + *index = (typeof(*index)) { + .offset = index->offset, + .packet_size = htobe64(packet_size), + .content_size = htobe64(content_size), + .timestamp_begin = htobe64(timestamp_begin), + .timestamp_end = htobe64(timestamp_end), + .events_discarded = htobe64(events_discarded), + .stream_id = htobe64(stream_id), + .stream_instance_id = htobe64(stream_instance_id), + .packet_seq_num = htobe64(packet_seq_num), + }; + error: return ret; } diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c index 514278d91..b64ad04aa 100644 --- a/src/common/ust-consumer/ust-consumer.c +++ b/src/common/ust-consumer/ust-consumer.c @@ -2424,62 +2424,69 @@ static int get_index_values(struct ctf_packet_index *index, struct ustctl_consumer_stream *ustream) { int ret; + uint64_t packet_size, content_size, timestamp_begin, timestamp_end, + events_discarded, stream_id, stream_instance_id, + packet_seq_num; - ret = ustctl_get_timestamp_begin(ustream, &index->timestamp_begin); + ret = ustctl_get_timestamp_begin(ustream, ×tamp_begin); if (ret < 0) { PERROR("ustctl_get_timestamp_begin"); goto error; } - index->timestamp_begin = htobe64(index->timestamp_begin); - ret = ustctl_get_timestamp_end(ustream, &index->timestamp_end); + ret = ustctl_get_timestamp_end(ustream, ×tamp_end); if (ret < 0) { PERROR("ustctl_get_timestamp_end"); goto error; } - index->timestamp_end = htobe64(index->timestamp_end); - ret = ustctl_get_events_discarded(ustream, &index->events_discarded); + ret = ustctl_get_events_discarded(ustream, &events_discarded); if (ret < 0) { PERROR("ustctl_get_events_discarded"); goto error; } - index->events_discarded = htobe64(index->events_discarded); - ret = ustctl_get_content_size(ustream, &index->content_size); + ret = ustctl_get_content_size(ustream, &content_size); if (ret < 0) { PERROR("ustctl_get_content_size"); goto error; } - index->content_size = htobe64(index->content_size); - ret = ustctl_get_packet_size(ustream, &index->packet_size); + ret = ustctl_get_packet_size(ustream, &packet_size); if (ret < 0) { PERROR("ustctl_get_packet_size"); goto error; } - index->packet_size = htobe64(index->packet_size); - ret = ustctl_get_stream_id(ustream, &index->stream_id); + ret = ustctl_get_stream_id(ustream, &stream_id); if (ret < 0) { PERROR("ustctl_get_stream_id"); goto error; } - index->stream_id = htobe64(index->stream_id); - ret = ustctl_get_instance_id(ustream, &index->stream_instance_id); + ret = ustctl_get_instance_id(ustream, &stream_instance_id); if (ret < 0) { PERROR("ustctl_get_instance_id"); goto error; } - index->stream_instance_id = htobe64(index->stream_instance_id); - ret = ustctl_get_sequence_number(ustream, &index->packet_seq_num); + ret = ustctl_get_sequence_number(ustream, &packet_seq_num); if (ret < 0) { PERROR("ustctl_get_sequence_number"); goto error; } - index->packet_seq_num = htobe64(index->packet_seq_num); + + *index = (typeof(*index)) { + .offset = index->offset, + .packet_size = htobe64(packet_size), + .content_size = htobe64(content_size), + .timestamp_begin = htobe64(timestamp_begin), + .timestamp_end = htobe64(timestamp_end), + .events_discarded = htobe64(events_discarded), + .stream_id = htobe64(stream_id), + .stream_instance_id = htobe64(stream_instance_id), + .packet_seq_num = htobe64(packet_seq_num), + }; error: return ret; -- 2.34.1