Make access to shared memory atomic

Use lock and unlock to make sure that all incrementing and decrementing of the reference count in the shared memory section is atomic. This ensures that there are not unusual timing conditions that could see 2 callers trying to update the reference count at the same time, which could result in the count never returning to zero. If that happened, then the filesystems would never be restored to read-only. (this is really just relevant to nanobsd) (note that shmop_* calls in php do not do any locking themselves - callers must coordinate their own access to the shared memory section)
I also made the number stored in the shared memory be an ordinary integer followed by null chars to pad out the rest of the shared memory section. This makes the numbers stored look normal, and ensures that there is no other rubbish left in memory after them to cause problems on reading.

1 files changed, 30 insertions, 13 deletions

diff --git a/etc/inc/util.inc b/etc/inc/util.inc
index b720d29..2dad476 100644
--- a/etc/inc/util.inc
+++ b/etc/inc/util.inc
@@ -187,22 +187,34 @@ function send_multiple_events($cmds) {
 }
 
 function refcount_init($reference) {
-	$shmid = @shmop_open($reference, "c", 0644, 10);
-	@shmop_write($shmid, 0, 0);
-	@shmop_close($shmid);
+	/* Take out a lock while creating the shared memory, just in case something else tries at the same time. */
+	$shm_lck_filename = "shm$reference";
+	$shm_lck = lock($shm_lck_filename, LOCK_EX);
+	$shm_size = 10;
+	$shmid = shmop_open($reference, "c", 0644, $shm_size);
+	shmop_write($shmid, str_pad("0", 10, "\x0", STR_PAD_RIGHT), 0);
+	shmop_close($shmid);
+	unlock($shm_lck);
 }
 
 function refcount_reference($reference) {
 	try {
-		$shmid = @shmop_open($reference, "w", 0644, 10);
+		/* The first time through the shared memory will not be there. */
+		/* So suppress the warning here and handle the init in the if. */
+		$shmid = @shmop_open($reference, "w", 0, 0);
 		if (!$shmid) {
 			refcount_init($reference);
-			$shmid = @shmop_open($reference, "w", 0, 0);
+			$shmid = shmop_open($reference, "w", 0, 0);
 		}
-		$shm_data = @shmop_read($shmid, 0, 10);
-		$shm_data = str_pad(intval($shm_data) + 1, 4, "0", STR_PAD_LEFT);
-		@shmop_write($shmid, $shm_data, 0);
-		@shmop_close($shmid);
+		/* Take out a lock across the shared memory read, increment, write sequence to make it atomic. */
+		$shm_lck_filename = "shm$reference";
+		$shm_lck = lock($shm_lck_filename, LOCK_EX);
+		$shm_size = shmop_size($shmid);
+		$shm_data = shmop_read($shmid, 0, $shm_size);
+		$shm_data = intval($shm_data) + 1;
+		shmop_write($shmid, str_pad($shm_data, $shm_size, "\x0", STR_PAD_RIGHT), 0);
+		shmop_close($shmid);
+		unlock($shm_lck);
 	} catch (Exception $e) {
 		log_error($e->getMessage());
 	}
@@ -214,14 +226,19 @@ function refcount_unreference($reference) {
 	try {
 		/* We assume that the shared memory exists. */
 		$shmid = @shmop_open($reference, "w", 0, 0);
-		$shm_data = @shmop_read($shmid, 0, 10);
+		/* Take out a lock across the shared memory read, decrement, write sequence to make it atomic. */
+		$shm_lck_filename = "shm$reference";
+		$shm_lck = lock($shm_lck_filename, LOCK_EX);
+		$shm_size = shmop_size($shmid);
+		$shm_data = @shmop_read($shmid, 0, $shm_size);
 		$shm_data = intval($shm_data) - 1;
 		if ($shm_data < 0) {
 			//debug_backtrace();
 			log_error(sprintf(gettext("Reference %s is going negative, not doing unreference."), $reference));
 		} else
-			@shmop_write($shmid, str_pad($shm_data, 4, "0", STR_PAD_LEFT), 0);
-		@shmop_close($shmid);
+			shmop_write($shmid, str_pad($shm_data, $shm_size, "\x0", STR_PAD_RIGHT), 0);
+		shmop_close($shmid);
+		unlock($shm_lck);
 	} catch (Exception $e) {
 		log_error($e->getMessage());
 	}
@@ -1832,4 +1849,4 @@ function setup_library_paths() {
 	}
 }
 
-?>
+?>
\ No newline at end of file