aft: Avoid stale pointer pointer reference.

We can't rely on the ->hash and ->path pointers of struct
status_item_ls_data because they both become stale when the audio
file table gets remapped. This happens for example when a new audio
file is added while another audio file is currently open. A subsequent
addblob command then triggers a re-initialization of the status items,
which reads from the stale pointer location.

This usually results in garbage in the stat output, but can also lead
to a segfault or worse. Avoid this from happening by creating a copy
of the hash and the path.

This bug has been present for a long time. It was hard to debug
because often no invalid memory accesses occur, hence valgrind does
not complain.

diff --git a/aft.c b/aft.c
index c04d4f9c99e89afb451afe94be7fcc64461b15e1..e24a5fbfe1c490d1a79b691484f9d43fd60a6266 100644 (file)
--- a/aft.c
+++ b/aft.c
@@ -1039,13 +1039,27 @@ again:
        ret = score_get_best(&current_aft_row, &d->score);
        if (ret < 0)
                return ret;
+       /*
+        * get_hash_of_row() and get_audio_file_path_of_row() initialize
+        * d->hash and d->path to point to memory-mapped files. These pointers
+        * become stale after a new audio file has been added or after the
+        * server process received SIGHUP. For in both cases libosl unmaps and
+        * remaps the underlying database files, and this remapping may well
+        * change the starting address of the mapping. To avoid stale pointer
+        * references we create copies on the heap.
+        */
+       free(d->hash);
        ret = get_hash_of_row(current_aft_row, &d->hash);
        if (ret < 0)
                return ret;
+       d->hash = (unsigned char *)para_strdup((char *)d->hash);
+       free(d->path);
        ret = get_audio_file_path_of_row(current_aft_row, &d->path);
        if (ret < 0)
                return ret;
        PARA_NOTICE_LOG("%s\n", d->path);
+       d->path = para_strdup(d->path);
+
        ret = get_afsi_object_of_row(current_aft_row, &afsi_obj);
        if (ret < 0)
                return ret;
@@ -2579,7 +2593,8 @@ static int aft_event_handler(enum afs_events event, struct para_buffer *pb,
                ret = get_audio_file_path_of_row(current_aft_row, &path);
                if (ret < 0)
                        return ret;
-               status_item_ls_data.path = path;
+               free(status_item_ls_data.path);
+               status_item_ls_data.path = para_strdup(path);
                make_status_items();
                return 1;
        } case AFHI_CHANGE: {