From b9c789b8b34a45a1572590a9ef4c8aa1306b4c63 Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Thu, 26 Oct 2023 14:50:43 +1100
Subject: [PATCH 1/4] Only remove the lock file if we managed to lock it

The lock file was being removed when we hadn't successfully locked
it which defeated the purpose of the lockfile.  Adjust cleanup_lockfile
such that it only unlinks the lockfile if we have successfully locked
the lockfile and it is still active (lockfile != NULL).
---
 bin/named/os.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/bin/named/os.c b/bin/named/os.c
index 858ccbff3f..ee8a094e5b 100644
--- a/bin/named/os.c
+++ b/bin/named/os.c
@@ -706,17 +706,19 @@ cleanup_pidfile(void) {
 }
 
 static void
-cleanup_lockfile(void) {
+cleanup_lockfile(bool unlink_lockfile) {
 	if (singletonfd != -1) {
 		close(singletonfd);
 		singletonfd = -1;
 	}
 
 	if (lockfile != NULL) {
-		int n = unlink(lockfile);
-		if (n == -1 && errno != ENOENT) {
-			named_main_earlywarning("unlink '%s': failed",
-						lockfile);
+		if (unlink_lockfile) {
+			int n = unlink(lockfile);
+			if (n == -1 && errno != ENOENT) {
+				named_main_earlywarning("unlink '%s': failed",
+							lockfile);
+			}
 		}
 		free(lockfile);
 		lockfile = NULL;
@@ -932,7 +934,7 @@ named_os_issingleton(const char *filename) {
 		if (ret == -1) {
 			named_main_earlywarning("couldn't create '%s'",
 						filename);
-			cleanup_lockfile();
+			cleanup_lockfile(false);
 			return (false);
 		}
 	}
@@ -944,7 +946,7 @@ named_os_issingleton(const char *filename) {
 	singletonfd = open(filename, O_WRONLY | O_CREAT,
 			   S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 	if (singletonfd == -1) {
-		cleanup_lockfile();
+		cleanup_lockfile(false);
 		return (false);
 	}
 
@@ -956,8 +958,7 @@ named_os_issingleton(const char *filename) {
 
 	/* Non-blocking (does not wait for lock) */
 	if (fcntl(singletonfd, F_SETLK, &lock) == -1) {
-		close(singletonfd);
-		singletonfd = -1;
+		cleanup_lockfile(false);
 		return (false);
 	}
 
@@ -968,7 +969,7 @@ void
 named_os_shutdown(void) {
 	closelog();
 	cleanup_pidfile();
-	cleanup_lockfile();
+	cleanup_lockfile(true);
 }
 
 void

From 811c9ee7d17927b534f9c83b20395389e2d145b2 Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Thu, 26 Oct 2023 15:07:58 +1100
Subject: [PATCH 2/4] Check that the lock file was not removed too early

When named fails to starts due to not being able to obtain
a lock on the lock file that lock file should remain.  Check
that the lock file exists before and after the attempt to
start a second instance of named.
---
 bin/tests/system/runtime/tests.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/bin/tests/system/runtime/tests.sh b/bin/tests/system/runtime/tests.sh
index 058bf39c6e..b9fd9ea4d0 100644
--- a/bin/tests/system/runtime/tests.sh
+++ b/bin/tests/system/runtime/tests.sh
@@ -81,12 +81,14 @@ status=$((status+ret))
 n=$((n+1))
 echo_i "verifying that named checks for conflicting named processes ($n)"
 ret=0
+test -f ns2/named.lock || ret=1
 testpid=$(run_named ns2 named$n.run -c named-alt2.conf -D runtime-ns2-extra-2 -X named.lock)
 test -n "$testpid" || ret=1
 retry_quiet 10 check_named_log "another named process" ns2/named$n.run || ret=1
 test -n "$testpid" && retry_quiet 10 check_pid $testpid || ret=1
 test -n "$testpid" && kill -15 $testpid > kill$n.out 2>&1 && ret=1
 test -n "$testpid" && retry_quiet 10 check_pid $testpid || ret=1
+test -f ns2/named.lock || ret=1
 if [ $ret -ne 0 ]; then echo_i "failed"; fi
 status=$((status+ret))
 

From a8613372c95bf9c8608dd1bf4c67f2089be89ba7 Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Thu, 26 Oct 2023 15:07:58 +1100
Subject: [PATCH 3/4] Add CHANGES note for [GL #4387]

---
 CHANGES | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/CHANGES b/CHANGES
index 06e567b388..46bca3cfb2 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+6274.	[bug]		The 'lock-file' file was being removed when it
+			shouldn't have been making it ineffective if named was
+			started 3 or more times. [GL #4387]
+
 6273.	[bug]		Don't reuse the existing TCP streams in dns_xfrin, so
 			parallel TCP transfers works again. [GL #4379]
 

From c1b8279ebbdc7f80857feef8a67037e6a7d75131 Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Thu, 26 Oct 2023 16:14:02 +1100
Subject: [PATCH 4/4] Add release note for [GL #4387]

---
 doc/notes/notes-current.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst
index 7491e3dc7f..4bec73ae31 100644
--- a/doc/notes/notes-current.rst
+++ b/doc/notes/notes-current.rst
@@ -61,6 +61,10 @@ Bug Fixes
   DNSSEC records, it was scheduled to be resigning. This unwanted behavior
   has been fixed. :gl:`#4350`
 
+- The :any:`lock-file` file was being removed when it shouldn't
+  have been making it ineffective if named was started 3 or more
+  times. :gl:`#4387`
+
 Known Issues
 ~~~~~~~~~~~~