From fcf104173870de4678c8f69e33039d39193c939a Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Mon, 22 Sep 2014 12:59:11 -0400 Subject: [PATCH] Fix: Don't assume that PROT_WRITE grants read permissions MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The prot flag passed to mmap() is set to PROT_WRITE when the O_RDWR access flags is used. This assumes that PROT_WRITE grants read permissions on the mmap'ed region. While this is true on x86, this causes a segmentation fault on SPARC and, presumably, other architectures implementing strict access permissions. CTF Writer needs read permissions since the unaligned integer writes may use load instructions as the value is read back to perform the required shifting/masking is performed in the _bt_bitfield_write_* macros. Signed-off-by: Jérémie Galarneau --- formats/ctf/ctf.c | 14 ++++--- formats/lttng-live/lttng-live-comm.c | 6 ++- include/babeltrace/ctf/types.h | 56 ++++++++++++++-------------- 3 files changed, 41 insertions(+), 35 deletions(-) diff --git a/formats/ctf/ctf.c b/formats/ctf/ctf.c index 8318a96a..58b54889 100644 --- a/formats/ctf/ctf.c +++ b/formats/ctf/ctf.c @@ -763,7 +763,7 @@ int ctf_init_pos(struct ctf_stream_pos *pos, struct bt_trace_descriptor *trace, pos->parent.trace = trace; break; case O_RDWR: - pos->prot = PROT_WRITE; /* Write has priority */ + pos->prot = PROT_READ | PROT_WRITE; pos->flags = MAP_SHARED; pos->parent.rw_table = write_dispatch_table; pos->parent.event_cb = ctf_write_event; @@ -779,7 +779,7 @@ int ctf_init_pos(struct ctf_stream_pos *pos, struct bt_trace_descriptor *trace, int ctf_fini_pos(struct ctf_stream_pos *pos) { - if (pos->prot == PROT_WRITE && pos->content_size_loc) + if ((pos->prot & PROT_WRITE) && pos->content_size_loc) *pos->content_size_loc = pos->offset; if (pos->base_mma) { int ret; @@ -873,7 +873,7 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, size_t index, int whence) assert(0); } - if (pos->prot == PROT_WRITE && pos->content_size_loc) + if ((pos->prot & PROT_WRITE) && pos->content_size_loc) *pos->content_size_loc = pos->offset; if (pos->base_mma) { @@ -891,7 +891,7 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, size_t index, int whence) * The caller should never ask for ctf_move_pos across packets, * except to get exactly at the beginning of the next packet. */ - if (pos->prot == PROT_WRITE) { + if (pos->prot & PROT_WRITE) { switch (whence) { case SEEK_CUR: /* The writer will add padding */ @@ -1001,12 +1001,14 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, size_t index, int whence) } /* update trace_packet_header and stream_packet_context */ - if (pos->prot != PROT_WRITE && file_stream->parent.trace_packet_header) { + if (!(pos->prot & PROT_WRITE) && + file_stream->parent.trace_packet_header) { /* Read packet header */ ret = generic_rw(&pos->parent, &file_stream->parent.trace_packet_header->p); assert(!ret); } - if (pos->prot != PROT_WRITE && file_stream->parent.stream_packet_context) { + if (!(pos->prot & PROT_WRITE) && + file_stream->parent.stream_packet_context) { /* Read packet context */ ret = generic_rw(&pos->parent, &file_stream->parent.stream_packet_context->p); assert(!ret); diff --git a/formats/lttng-live/lttng-live-comm.c b/formats/lttng-live/lttng-live-comm.c index 0cd62f98..238cafdd 100644 --- a/formats/lttng-live/lttng-live-comm.c +++ b/formats/lttng-live/lttng-live-comm.c @@ -1065,7 +1065,8 @@ void read_packet_header(struct ctf_stream_pos *pos, int ret; /* update trace_packet_header and stream_packet_context */ - if (pos->prot != PROT_WRITE && file_stream->parent.trace_packet_header) { + if (!(pos->prot & PROT_WRITE) && + file_stream->parent.trace_packet_header) { /* Read packet header */ ret = generic_rw(&pos->parent, &file_stream->parent.trace_packet_header->p); @@ -1076,7 +1077,8 @@ void read_packet_header(struct ctf_stream_pos *pos, goto end; } } - if (pos->prot != PROT_WRITE && file_stream->parent.stream_packet_context) { + if (!(pos->prot & PROT_WRITE) && + file_stream->parent.stream_packet_context) { /* Read packet context */ ret = generic_rw(&pos->parent, &file_stream->parent.stream_packet_context->p); diff --git a/include/babeltrace/ctf/types.h b/include/babeltrace/ctf/types.h index 67b24ee4..c336632f 100644 --- a/include/babeltrace/ctf/types.h +++ b/include/babeltrace/ctf/types.h @@ -132,6 +132,28 @@ int ctf_init_pos(struct ctf_stream_pos *pos, struct bt_trace_descriptor *trace, int fd, int open_flags); int ctf_fini_pos(struct ctf_stream_pos *pos); +static inline +int ctf_pos_access_ok(struct ctf_stream_pos *pos, uint64_t bit_len) +{ + uint64_t max_len; + + if (unlikely(pos->offset == EOF)) + return 0; + if (pos->prot == PROT_READ) { + /* + * Reads may only reach up to the "content_size", + * regardless of the packet_size. + */ + max_len = pos->content_size; + } else { + /* Writes may take place up to the end of the packet. */ + max_len = pos->packet_size; + } + if (unlikely(pos->offset + bit_len > max_len)) + return 0; + return 1; +} + /* * move_pos - move position of a relative bit offset * @@ -142,21 +164,17 @@ int ctf_fini_pos(struct ctf_stream_pos *pos); static inline int ctf_move_pos(struct ctf_stream_pos *pos, uint64_t bit_offset) { - uint64_t max_len; + int ret = 0; printf_debug("ctf_move_pos test EOF: %" PRId64 "\n", pos->offset); - if (unlikely(pos->offset == EOF)) - return 0; - if (pos->prot == PROT_READ) - max_len = pos->content_size; - else - max_len = pos->packet_size; - if (unlikely(pos->offset + bit_offset > max_len)) - return 0; - + ret = ctf_pos_access_ok(pos, bit_offset); + if (!ret) { + goto end; + } pos->offset += bit_offset; printf_debug("ctf_move_pos after increment: %" PRId64 "\n", pos->offset); - return 1; +end: + return ret; } /* @@ -207,22 +225,6 @@ void ctf_pos_pad_packet(struct ctf_stream_pos *pos) ctf_packet_seek(&pos->parent, 0, SEEK_CUR); } -static inline -int ctf_pos_access_ok(struct ctf_stream_pos *pos, uint64_t bit_len) -{ - uint64_t max_len; - - if (unlikely(pos->offset == EOF)) - return 0; - if (pos->prot == PROT_READ) - max_len = pos->content_size; - else - max_len = pos->packet_size; - if (unlikely(pos->offset + bit_len > max_len)) - return 0; - return 1; -} - /* * Update the stream position to the current event. This moves to * the next packet if we are located at the end of the current packet. -- 2.34.1