View Issue Details

IDProjectCategoryView StatusLast Update
0000604LDMud 3.2Implementationpublic2018-01-29 22:57
Reporterwillem Assigned Tofufu  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.2.15 
Summary0000604: erq_args is built incorrectly
DescriptionThis is actually for driver 3.2.15, but mantis doesn't have that version as an option. I believe the bug is also present on 3.3.

eval_arg() in main.c doesn't correctly build erq_args.

Reproduction:
./ldmud --erq "/home/mud/bin/erq --execdir /home/mud/libexec"
The ERQ daemon will not run.

An strace reveals:

execve("/home/mud/bin/erq", ["erq", "--forked", "--execdir", "/home/mud/libexec", "\7", 0x2000000d, 0x6d6f682f, 0x756d2f65, 0x622f3264, 0x652f6e69, 0x2d007172, 0x6578652d, 0x72696463, 0x6f682f00, 0x6d2f656d, 0x2f326475, ...], [/* 28 vars */]) = -1 EFAULT (Bad address)
Additional InformationMy fix was to change:

/* Ensure termination */
*cp++ = '\0';

To:

/* Ensure termination */
if (*cp != '\0') *cp++ = '\0';
TagsNo tags attached.

Activities

zesstra

2009-01-30 02:58

administrator   ~0000940

I can't reproduce this on my machine and the code in eval_args actually looks good.
I wonder if your erq_args was corrupted afterwards or if your compiler did some curious things?
Your patch should IMHO not change anything in the resulting erq_args as the only difference is, that the current code may replace a \0 by \0.

fufu

2009-01-30 08:16

manager   ~0000941

The bug is valid, and so is the fix - the effect of the change is that the 'cp' pointer does not get incremented beyond the final \0 of the command line argument anymore. I'll take this one.

fufu

2009-01-30 09:10

manager   ~0000942

Fixed in rev 2515. I'm still thinking about whether and how best to add a test for this.

2009-01-30 10:08

 

test.patch (3,626 bytes)   
commit 5fd1ef84ec3958179999856824beb15b711b1c3c
Author: Bertram Felgenhauer <int-e@gmx.de>
Date:   Fri Jan 30 16:02:19 2009 +0100

    (tests) refactor run.sh script

diff --git a/test/run.sh b/test/run.sh
index e9c1de7..87686cf 100755
--- a/test/run.sh
+++ b/test/run.sh
@@ -11,36 +11,41 @@ mkdir -p log
 
 FAILED=""
 
+DRIVER_DEFAULTS="-u-1 -E 0 --no-compat -e -N --cleanup-time -1 --reset-time -1
+    --max-array 0 --max-callouts 0 --max-bytes 0 --max-file 0 -s-1
+    -sv-1 --max-malloc 0 --min-malloc 0 -ru0 -rm0 -rs0 --no-strict-euids
+    --no-wizlist-file --check-refcounts --check-state 2 --access-file none
+    --access-log none -f test 65432"
+
+export DRIVER DRIVER_DEFAULTS
+
 for testdir in ${@:-t-*}
 do
-    if [ -d "$testdir" ]
-    then
-	${DRIVER} -u-1 -E 0 -Mmaster -m"$testdir" --debug-file "../log/result.$testdir.log" --no-compat -e -N \
-              --cleanup-time -1 --reset-time -1 --max-array 0 \
-	      --max-callouts 0 --max-bytes 0 --max-file 0 \
-	      -s-1 -sv-1 --max-malloc 0 --min-malloc 0 \
-	      -ru0 -rm0 -rs0 --no-strict-euids --no-wizlist-file \
-	      --check-refcounts --check-state 2 \
-	      --access-file none --access-log none \
-	      -f test 65432 > /dev/null || { echo "Test $testdir FAILED."; FAILED="${FAILED}\n\t$testdir"; }
-    elif [ -r "$testdir" ]
+    if [ -d "${testdir}" ]
     then
-	${DRIVER} -u-1 -E 0 -M"$testdir" -m. --debug-file "./log/result.$testdir.log" --no-compat -e -N \
-              --cleanup-time -1 --reset-time -1 --max-array 0 \
-	      --max-callouts 0 --max-bytes 0 --max-file 0 \
-	      -s-1 -sv-1 --max-malloc 0 --min-malloc 0 \
-	      -ru0 -rm0 -rs0 --no-strict-euids --no-wizlist-file \
-	      --check-refcounts --check-state 2 \
-	      --access-file none --access-log none \
-	      -f test 65432 > /dev/null || { echo "Test $testdir FAILED."; FAILED="${FAILED}\n\t$testdir"; }
+	${DRIVER} ${DRIVER_DEFAULTS} -Mmaster -m"${testdir}" \
+              --debug-file "../log/result.${testdir}.log"  > /dev/null \
+        || { echo "Test ${testdir} FAILED."; FAILED="${FAILED}\n\t${testdir}"; }
+        continue
     fi
+    case ${testdir} in
+    *.c)
+	${DRIVER} ${DRIVER_DEFAULTS} -M"${testdir}" -m. \
+              --debug-file "./log/result.${testdir}.log" > /dev/null \
+        || { echo "Test ${testdir} FAILED."; FAILED="${FAILED}\n\t${testdir}"; }
+    ;;
+    *.sh)
+        /bin/sh ./${testdir} 2>&1 > "./log/result.${testdir}.log" \
+        || { echo "Test ${testdir} FAILED."; FAILED="${FAILED}\n\t${testdir}"; }
+    ;;
+    esac
 done
 
-if [ -z "$FAILED" ]
+if [ -z "${FAILED}" ]
 then
     echo "Tests run successfully."
 else
     echo "The following tests FAILED:"
-    echo -e "$FAILED"
+    echo -e "${FAILED}"
     exit 1
 fi

==============================================================================
commit 190bfcdd839f653b3028760d14d497a290e2df6b
Author: Bertram Felgenhauer <int-e@gmx.de>
Date:   Fri Jan 30 16:03:34 2009 +0100

    add test for #604

diff --git a/test/generic/master.c b/test/generic/master.c
new file mode 100644
index 0000000..c315229
--- /dev/null
+++ b/test/generic/master.c
@@ -0,0 +1,8 @@
+#include "/inc/base.inc"
+#include "/inc/gc.inc"
+
+string *epilog(int eflag)
+{
+    shutdown();
+    return 0;
+}
diff --git a/test/t-0000640.sh b/test/t-0000640.sh
new file mode 100644
index 0000000..f457dc9
--- /dev/null
+++ b/test/t-0000640.sh
@@ -0,0 +1,5 @@
+ulimit -c 0
+for extra in 0 01 012 0123 01234 012345 0123456 01234567; do
+    ${DRIVER}  --erq "/bin/true ${extra}" ${DRIVER_DEFAULTS/ -N / } \
+        -Mgeneric/master -m. --debug-file=/dev/null || exit 1
+done
test.patch (3,626 bytes)   

fufu

2009-01-30 10:09

manager   ~0000943

I've refactored the 'run.sh' script to allow running shell scripts, and created a test that segfaults for me due to this bug.

Gnomi, do those changes look good to you?

Gnomi

2009-01-30 17:38

manager   ~0000944

I think 'continue' isn't a POSIX shell command, so the case statement should be put in a else block. And t-0000640.sh should output a line or two about that it is run. Despite that it looks fine to me.

fufu

2009-01-30 18:06

manager   ~0000945

Thanks. More annoyingly, ${FOO/a/b} is not available in the POSIX shell either, according to http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html

fufu

2009-01-31 13:03

manager   ~0000946

Test added in rev. 2517
The fix will be in 3.3.719

Issue History

Date Modified Username Field Change
2009-01-29 18:38 willem New Issue
2009-01-30 02:58 zesstra Note Added: 0000940
2009-01-30 02:58 zesstra Status new => acknowledged
2009-01-30 05:57 zesstra Project LDMud => LDMud 3.2
2009-01-30 05:58 zesstra Product Version 3.2.13 => 3.2.15
2009-01-30 08:16 fufu Note Added: 0000941
2009-01-30 08:16 fufu Status acknowledged => assigned
2009-01-30 08:16 fufu Assigned To => fufu
2009-01-30 09:10 fufu Note Added: 0000942
2009-01-30 10:08 fufu File Added: test.patch
2009-01-30 10:09 fufu Note Added: 0000943
2009-01-30 17:38 Gnomi Note Added: 0000944
2009-01-30 18:06 fufu Note Added: 0000945
2009-01-31 13:03 fufu Note Added: 0000946
2009-01-31 13:03 fufu Status assigned => resolved
2009-01-31 13:03 fufu Resolution open => fixed
2010-11-16 10:42 Gnomi Source_changeset_attached => ldmud.git master-3.2 50adedb8
2018-01-29 19:59 Gnomi Source_changeset_attached => ldmud.git master-3.2 50adedb8
2018-01-29 22:57 Gnomi Source_changeset_attached => ldmud.git master-3.2 50adedb8