From: Martin Schwenke <mschwenke@ddn.com>
Date: Wed, 20 Nov 2024 03:37:09 +0000 (+1100)
Subject: ctdb-common: Map ENOENT for a missing event script to ENOEXEC
X-Git-Tag: tdb-1.4.13~492
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=fee31b6cb2b8f7dd111bdd9d2ff5479c31cbca37;p=thirdparty%2Fsamba.git

ctdb-common: Map ENOENT for a missing event script to ENOEXEC

This handles the case where an event script is disabled by unlinking,
while an event is being run, after the script list has been created.
Without this change the script will fail.  With this change the script
will be marked as DISABLED.  See the comment added by this commit for
more details.

Add a testcase to simulate the race, using an event script to disable
subsequent ones.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15755

Signed-off-by: Martin Schwenke <mschwenke@ddn.com>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

Autobuild-User(master): Amitay Isaacs <amitay@samba.org>
Autobuild-Date(master): Thu Nov 21 01:42:09 UTC 2024 on atb-devel-224
---

diff --git a/ctdb/common/run_event.c b/ctdb/common/run_event.c
index d283664e2cf..30369eeff22 100644
--- a/ctdb/common/run_event.c
+++ b/ctdb/common/run_event.c
@@ -268,8 +268,27 @@ static int run_event_script_status(struct run_event_script *script)
 	if (script->result.sig > 0) {
 		ret = -EINTR;
 	} else if (script->result.err > 0) {
-		if (script->result.err == EACCES) {
-			/* Map EACCESS to ENOEXEC */
+		if (script->result.err == EACCES ||
+		    script->result.err == ENOENT) {
+			/*
+			 * Map EACCESS/ENOENT to ENOEXEC
+			 *
+			 * ENOENT: Disabling a standard event script
+			 * by removing its symlink can result in
+			 * ENOENT.  This happens when the script list
+			 * is built while the link exists, but the
+			 * link is removed before the attempt to run
+			 * it.  Map it to ENOEXEC (which causes a
+			 * script to be shown as DISABLED).  This
+			 * makes it impossible to distinguish a
+			 * removed symlink from a dangling
+			 * symlink... but the latter can just be
+			 * defined as disabled.  It should be rare
+			 * because it shouldn't happen if event
+			 * scripts are properly managed.  If someone
+			 * is doing weird things then they can easily
+			 * debug such issues by looking at the link.
+			 */
 			ret = -ENOEXEC;
 		} else {
 			ret = -script->result.err;
diff --git a/ctdb/tests/UNIT/eventd/etc-ctdb/share/events/data/01.dummy.script b/ctdb/tests/UNIT/eventd/etc-ctdb/share/events/data/01.dummy.script
index 9c56f5b968b..b7b5945714c 100755
--- a/ctdb/tests/UNIT/eventd/etc-ctdb/share/events/data/01.dummy.script
+++ b/ctdb/tests/UNIT/eventd/etc-ctdb/share/events/data/01.dummy.script
@@ -2,5 +2,9 @@
 
 case "$1" in
 "failure") exit 1 ;;
+"disablehack")
+	ctdb-event script disable data 02.disabled
+	ctdb-event script disable data 03.notalink
+	;;
 *) exit 0 ;;
 esac
diff --git a/ctdb/tests/UNIT/eventd/eventd_009.sh b/ctdb/tests/UNIT/eventd/eventd_009.sh
index 39e5cd658cc..8ac14d952fe 100755
--- a/ctdb/tests/UNIT/eventd/eventd_009.sh
+++ b/ctdb/tests/UNIT/eventd/eventd_009.sh
@@ -153,3 +153,40 @@ ok <<EOF
 * 03.notalink
 EOF
 simple_test script list data
+
+#
+# Test disabling of scripts after the script list has been
+# built. Normally this would be an admin racing with eventd instead of
+# one script disabling subsequent ones.
+#
+
+# First enable all scripts - this might repeat some previous stuff
+ok_null
+simple_test script enable data 01.dummy
+ok_null
+simple_test script enable data 02.disabled
+ok_null
+simple_test script enable data 03.notalink
+
+# Confirm expected state
+ok <<EOF
+* 01.dummy
+* 02.disabled
+
+* 03.notalink
+EOF
+simple_test script list data
+
+# Now run the event that disables the subsequent scripts:
+# - 02.disabled has its link removed
+# - 03.notalink effectively has "chmod -x" applied
+ok_null
+simple_test run 10 data disablehack
+
+# Confirm that both subsequent 
+ok <<EOF
+01.dummy             OK         DURATION DATETIME
+02.disabled          DISABLED  
+03.notalink          DISABLED  
+EOF
+simple_test status data disablehack