Fix: SIGBUS error on reading past a file's end in mmap'ed region

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/plugins/ctf/fs/data-stream.c b/plugins/ctf/fs/data-stream.c
index 4c3ab29a33e94bc56a6903dd6226efe541c0181e..6b5005199ee5f3b26a241f6fd64a6d9087192421 100644 (file)
--- a/plugins/ctf/fs/data-stream.c
+++ b/plugins/ctf/fs/data-stream.c
@@ -1,9 +1,8 @@
 /*
  * Copyright 2016 - Philippe Proulx <pproulx@efficios.com>
+ * Copyright 2016 - Jérémie Galarneau <jeremie.galarneau@efficios.com>
  * Copyright 2010-2011 - EfficiOS Inc. and Linux Foundation
  *
- * Some functions are based on older functions written by Mathieu Desnoyers.
- *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
@@ -61,12 +60,12 @@ void ctf_fs_stream_destroy(struct ctf_fs_stream *stream)
 
 static size_t remaining_mmap_bytes(struct ctf_fs_stream *stream)
 {
-       return stream->mmap_offset + stream->mmap_len -
-               stream->request_offset;
+       return stream->mmap_valid_len - stream->request_offset;
 }
 
 static int stream_munmap(struct ctf_fs_stream *stream)
 {
+       int ret = 0;
        struct ctf_fs_component *ctf_fs = stream->file->ctf_fs;
 
        if (munmap(stream->mmap_addr, stream->mmap_len)) {
@@ -74,16 +73,17 @@ static int stream_munmap(struct ctf_fs_stream *stream)
                        stream->mmap_addr, stream->mmap_len,
                        stream->file->path->str, stream->file->fp,
                        strerror(errno));
-               return -1;
+               ret = -1;
+               goto end;
        }
-
-       return 0;
+end:
+       return ret;
 }
 
 static int mmap_next(struct ctf_fs_stream *stream)
 {
-       struct ctf_fs_component *ctf_fs = stream->file->ctf_fs;
        int ret = 0;
+       struct ctf_fs_component *ctf_fs = stream->file->ctf_fs;
 
        /* Unmap old region */
        if (stream->mmap_addr) {
@@ -91,10 +91,15 @@ static int mmap_next(struct ctf_fs_stream *stream)
                        goto error;
                }
 
-               stream->mmap_offset += stream->mmap_len;
-               stream->request_offset = stream->mmap_offset;
+               stream->mmap_offset += stream->mmap_valid_len;
+               stream->request_offset = 0;
        }
 
+       stream->mmap_valid_len = MIN(stream->file->size - stream->mmap_offset,
+                       stream->mmap_max_len);
+       /* Round up to next page, assuming page size being a power of 2. */
+       stream->mmap_len = (stream->mmap_valid_len + ctf_fs->page_size - 1)
+                       & ~(ctf_fs->page_size - 1);
        /* Map new region */
        stream->mmap_addr = mmap((void *) 0, stream->mmap_len,
                        PROT_READ, MAP_PRIVATE, fileno(stream->file->fp),
@@ -140,7 +145,7 @@ static enum bt_ctf_notif_iter_medium_status medop_request_bytes(
        /* Check if we have at least one memory-mapped byte left */
        if (remaining_mmap_bytes(stream) == 0) {
                /* Are we at the end of the file? */
-               if (stream->request_offset == stream->file->size) {
+               if (stream->request_offset >= stream->file->size) {
                        PDBG("Reached end of file \"%s\" (%p)\n",
                                stream->file->path->str, stream->file->fp);
                        status = BT_CTF_NOTIF_ITER_MEDIUM_STATUS_EOF;
@@ -155,8 +160,7 @@ static enum bt_ctf_notif_iter_medium_status medop_request_bytes(
        }
 
        *buffer_sz = MIN(remaining_mmap_bytes(stream), request_sz);
-       *buffer_addr = ((uint8_t *) stream->mmap_addr) +
-               stream->request_offset - stream->mmap_offset;
+       *buffer_addr = ((uint8_t *) stream->mmap_addr) + stream->request_offset;
        stream->request_offset += *buffer_sz;
        goto end;
 
@@ -178,10 +182,10 @@ static struct bt_ctf_stream *medop_get_stream(
 
                PDBG("Creating stream out of stream class %" PRId64 "\n", id);
                fs_stream->stream = bt_ctf_stream_create(stream_class,
-                       fs_stream->file->path->str);
+                               fs_stream->file->path->str);
                if (!fs_stream->stream) {
                        PERR("Cannot create stream (stream class %" PRId64 ")\n",
-                               id);
+                                       id);
                }
        }
 
@@ -209,7 +213,7 @@ struct ctf_fs_stream *ctf_fs_stream_create(
        if (!stream->notif_iter) {
                goto error;
        }
-       stream->mmap_len = ctf_fs->page_size * 2048;
+       stream->mmap_max_len = ctf_fs->page_size * 2048;
        goto end;
 error:
        /* Do not touch "borrowed" file. */
@@ -222,12 +226,14 @@ end:
 
 enum bt_notification_iterator_status ctf_fs_data_stream_get_next_notification(
                struct ctf_fs_component *ctf_fs,
-               struct bt_notification **notification)
+               struct bt_notification **notification,
+               size_t stream_id)
 {
        enum bt_ctf_notif_iter_status status;
        enum bt_notification_iterator_status ret;
        /* FIXME, only iterating on one stream for the moment. */
-       struct ctf_fs_stream *stream = g_ptr_array_index(ctf_fs->streams, 0);
+       struct ctf_fs_stream *stream = g_ptr_array_index(ctf_fs->streams,
+                       stream_id);
 
        if (stream->end_reached) {
                status = BT_CTF_NOTIF_ITER_STATUS_EOF;

diff --git a/plugins/ctf/fs/data-stream.h b/plugins/ctf/fs/data-stream.h
index d6bb97fcbe6d7d3b0951c78cc66f41424a46e115..c567c13e2de3f89a108a1a3a61acb3752720b737 100644 (file)
--- a/plugins/ctf/fs/data-stream.h
+++ b/plugins/ctf/fs/data-stream.h
@@ -43,6 +43,7 @@ int ctf_fs_data_stream_open_streams(struct ctf_fs_component *ctf_fs);
 BT_HIDDEN
 enum bt_notification_iterator_status ctf_fs_data_stream_get_next_notification(
                struct ctf_fs_component *ctf_fs,
-               struct bt_notification **notification);
+               struct bt_notification **notification,
+               size_t stream_id);
 
 #endif /* CTF_FS_DATA_STREAM_H */

diff --git a/plugins/ctf/fs/fs.c b/plugins/ctf/fs/fs.c
index 419f601c32cd6778805b79c81cf7fa53729df8c4..bf834f3fe4dd1a804a9ddfadb045e671ea224b84 100644 (file)
--- a/plugins/ctf/fs/fs.c
+++ b/plugins/ctf/fs/fs.c
@@ -84,7 +84,7 @@ enum bt_notification_iterator_status ctf_fs_iterator_next(
        ctf_fs = bt_component_get_private_data(component);
        assert(ctf_fs);
 
-       ret = ctf_fs_data_stream_get_next_notification(ctf_fs, &notification);
+       ret = ctf_fs_data_stream_get_next_notification(ctf_fs, &notification, 0);
        if (ret || !notification) {
                goto end;
        }

diff --git a/plugins/ctf/fs/fs.h b/plugins/ctf/fs/fs.h
index 151e28eda5d5c2d8f8f801e9ff5523e8a1509e91..cf574c8b3c32462ec5bb98e0d8f1a541dea6308d 100644 (file)
--- a/plugins/ctf/fs/fs.h
+++ b/plugins/ctf/fs/fs.h
@@ -60,8 +60,18 @@ struct ctf_fs_stream {
        /* FIXME There should be many and ctf_fs_stream should not own them. */
        struct bt_ctf_notif_iter *notif_iter;
        void *mmap_addr;
+       /* Max length of chunk to mmap() when updating the current mapping. */
+       size_t mmap_max_len;
+       /* Length of the current mapping. */
        size_t mmap_len;
+       /* Length of the current mapping which *exists* in the backing file. */
+       size_t mmap_valid_len;
+       /* Offset in the file where the current mapping starts. */
        off_t mmap_offset;
+       /*
+        * Offset, in the current mapping, of the address to return on the next
+        * request.
+        */
        off_t request_offset;
        bool end_reached;
 };