Fix: Don't assume that PROT_WRITE grants read permissions

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 <jeremie.galarneau@efficios.com>

diff --git a/formats/ctf/ctf.c b/formats/ctf/ctf.c
index 8318a96a0781742837f9f4c754754d83c25d9f68..58b5488920d006e71aafe96b764e5f49ec7c5b6d 100644 (file)
--- 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 0cd62f9812b8d03a201f2f85572697f8a2666c72..238cafdd002360a51ccb82c72fb3addebec5dabd 100644 (file)
--- 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 67b24ee4aa940cf3185bff55c57c2a127b2f2904..c336632f7e6eb34b93190ae5e4d71c5f1a12d994 100644 (file)
--- 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.