From: Andre Noll <maan@tuebingen.mpg.de>
Date: Mon, 23 Mar 2020 20:19:14 +0000 (+0100)
Subject: Merge branch 'refs/heads/t/stale-pointer-fix'
X-Git-Tag: v0.6.3~21
X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=57a338b4f7a8346708636de18780eaec52750e96;hp=7e60f73979da2e033eff54fd46eef63e6b70e07c

Merge branch 'refs/heads/t/stale-pointer-fix'

This bug only triggered if the kernel changes the address of the memory
mapping of the audio file table after a file was added, and a subsequent
operation would access the then stale pointer.

Cooking for a week.

* refs/heads/t/stale-pointer-fix:
  Don't use strdup() to copy hash.
  aft: Avoid stale pointer pointer reference.
---

diff --git a/NEWS.md b/NEWS.md
index 15496bd2..34aad072 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -26,6 +26,8 @@ NEWS
   the correct duration also if ogg pages are missing in the file. This
   affects ogg/vorbis ogg/speex and ogg/opus.
 - Robustness improvements for para_mixer.
+- A fix for an old bug that could cause the server to crash or report
+  garbage in its status output.
 
 --------------------------------------
 0.6.2 (2018-06-30) "elastic diversity"
diff --git a/aft.c b/aft.c
index c04d4f9c..5595071f 100644
--- a/aft.c
+++ b/aft.c
@@ -1035,17 +1035,33 @@ int open_and_update_audio_file(struct audio_file_data *afd)
 	struct afsi_change_event_data aced;
 	struct osl_object map, chunk_table_obj;
 	struct ls_data *d = &status_item_ls_data;
+	unsigned char *tmp_hash;
 again:
 	ret = score_get_best(&current_aft_row, &d->score);
 	if (ret < 0)
 		return ret;
-	ret = get_hash_of_row(current_aft_row, &d->hash);
+	/*
+	 * get_hash_of_row() and get_audio_file_path_of_row() initialize
+	 * their pointer argument 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.
+	 */
+	ret = get_hash_of_row(current_aft_row, &tmp_hash);
 	if (ret < 0)
 		return ret;
+	if (!d->hash)
+		d->hash = para_malloc(HASH_SIZE);
+	memcpy(d->hash, tmp_hash, HASH_SIZE);
+	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 +2595,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: {