Skip to content

Commit e246a89

Browse files
committed
judgedaemon: Replace system and other command executions.
Introduce a safer and more readable variant that executes, logs and checks return values centrally.
1 parent 720b750 commit e246a89

File tree

1 file changed

+88
-64
lines changed

1 file changed

+88
-64
lines changed

judge/judgedaemon.main.php

Lines changed: 88 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,41 @@ function read_judgehostlog(int $numLines = 20) : string
290290
return trim(ob_get_clean());
291291
}
292292

293+
define('DONT_CARE', new class {});
294+
/**
295+
* Execute a shell command given an array of strings forming the command.
296+
* The command and all its arguments are automatically escaped.
297+
*
298+
* @param array $command_parts The command and its arguments (e.g., ['ls', '-l', '/tmp/']).
299+
* @param mixed $retval The (integer) variable to store the command's exit code.
300+
* @param bool $log_nonzero_exitcode Whether non-zero exit codes should be logged.
301+
*
302+
* @return bool Returns true on success (exit code 0), false otherwise.
303+
*/
304+
function run_command_safe(array $command_parts, &$retval = DONT_CARE, $log_nonzero_exitcode = true): bool
305+
{
306+
if (empty($command_parts)) {
307+
logmsg(LOG_WARNING, "Need at least the command that should be called.");
308+
$retval = -1;
309+
return false;
310+
}
311+
312+
$command = implode(' ', array_map('dj_escapeshellarg', $command_parts));
313+
314+
logmsg(LOG_DEBUG, "Executing command: $command");
315+
system($command, $retval_local);
316+
if ($retval !== DONT_CARE) $retval = $retval_local;
317+
318+
if ($retval_local !== 0) {
319+
if ($log_nonzero_exitcode) {
320+
logmsg(LOG_WARNING, "Command failed with exit code $retval_local: $command");
321+
}
322+
return false;
323+
}
324+
325+
return true;
326+
}
327+
293328
// Fetches a new executable from database if not cached already, and runs build script to compile executable.
294329
// Returns an array with
295330
// - absolute path to run script
@@ -351,12 +386,10 @@ function fetch_executable_internal(
351386
$execrunjurypath = $execbuilddir . '/runjury';
352387
if (!is_dir($execdir) || !file_exists($execdeploypath) ||
353388
($combined_run_compare && file_get_contents(LIBJUDGEDIR . '/run-interactive.sh')!==file_get_contents($execrunpath))) {
354-
system('rm -rf ' . dj_escapeshellarg($execdir) . ' ' . dj_escapeshellarg($execbuilddir), $retval);
355-
if ($retval !== 0) {
389+
if (!run_command_safe(['rm', '-rf', $execdir, $execbuilddir])) {
356390
disable('judgehost', 'hostname', $myhost, "Deleting '$execdir' or '$execbuilddir' was unsuccessful.");
357391
}
358-
system('mkdir -p ' . dj_escapeshellarg($execbuilddir), $retval);
359-
if ($retval !== 0) {
392+
if (!run_command_safe(['mkdir', '-p', $execbuilddir])) {
360393
disable('judgehost', 'hostname', $myhost, "Could not create directory '$execbuilddir'");
361394
}
362395

@@ -466,8 +499,7 @@ function fetch_executable_internal(
466499
putenv('SCRIPTMEMLIMIT=' . djconfig_get_value('script_memory_limit'));
467500
putenv('SCRIPTFILELIMIT=' . djconfig_get_value('script_filesize_limit'));
468501

469-
system(LIBJUDGEDIR . '/build_executable.sh ' . dj_escapeshellarg($execdir), $retval);
470-
if ($retval !== 0) {
502+
if (!run_command_safe([LIBJUDGEDIR . '/build_executable.sh', $execdir])) {
471503
return [null, "Failed to build executable in $execdir.", "$execdir/build.log"];
472504
}
473505
chmod($execrunpath, 0755);
@@ -528,7 +560,11 @@ function fetch_executable_internal(
528560
exit(1);
529561
}
530562

531-
$myhost = trim(`hostname | cut -d . -f 1`);
563+
$hostname = gethostname();
564+
if ($hostname === false) {
565+
error("Could not determine hostname.");
566+
}
567+
$myhost = explode('.', $hostname)[0];
532568
if (isset($options['daemonid'])) {
533569
if (preg_match('/^\d+$/', $options['daemonid'])) {
534570
$myhost = $myhost . "-" . $options['daemonid'];
@@ -659,9 +695,8 @@ function fetch_executable_internal(
659695

660696
// Check basic prerequisites for chroot at judgehost startup
661697
logmsg(LOG_INFO, "🔏 Executing chroot script: '".CHROOT_SCRIPT." check'");
662-
system(LIBJUDGEDIR.'/'.CHROOT_SCRIPT.' check', $retval);
663-
if ($retval!==0) {
664-
error("chroot validation check exited with exitcode $retval");
698+
if (!run_command_safe([LIBJUDGEDIR.'/'.CHROOT_SCRIPT, 'check'])) {
699+
error("chroot validation check failed");
665700
}
666701

667702
foreach ($endpoints as $id => $endpoint) {
@@ -753,8 +788,7 @@ function fetch_executable_internal(
753788
foreach ($candidateDirs as $d) {
754789
$cnt++;
755790
logmsg(LOG_INFO, " - deleting $d");
756-
system('rm -rf ' . dj_escapeshellarg($d), $retval);
757-
if ($retval !== 0) {
791+
if (!run_command_safe(['rm', '-rf', $d])) {
758792
logmsg(LOG_WARNING, "Deleting '$d' was unsuccessful.");
759793
}
760794
$after = disk_free_space(JUDGEDIR);
@@ -878,10 +912,7 @@ function fetch_executable_internal(
878912
$judgeTask['judgetaskid']
879913
);
880914

881-
$debug_cmd = implode(' ', array_map('dj_escapeshellarg',
882-
[$runpath, $workdir, $tmpfile]));
883-
system($debug_cmd, $retval);
884-
if ($retval !== 0) {
915+
if (!run_command_safe([$runpath, $workdir, $tmpfile])) {
885916
disable('run_script', 'run_script_id', $judgeTask['run_script_id'], "Running '$runpath' failed.");
886917
}
887918

@@ -950,8 +981,7 @@ function fetch_executable_internal(
950981
}
951982

952983

953-
system('mkdir -p ' . dj_escapeshellarg("$workdir/compile"), $retval);
954-
if ($retval !== 0) {
984+
if (!run_command_safe(['mkdir', '-p', "$workdir/compile"])) {
955985
error("Could not create '$workdir/compile'");
956986
}
957987

@@ -964,8 +994,7 @@ function fetch_executable_internal(
964994
if ($lastWorkdir !== $workdir) {
965995
// create chroot environment
966996
logmsg(LOG_INFO, " 🔒 Executing chroot script: '".CHROOT_SCRIPT." start'");
967-
system(LIBJUDGEDIR.'/'.CHROOT_SCRIPT.' start', $retval);
968-
if ($retval!==0) {
997+
if (!run_command_safe([LIBJUDGEDIR.'/'.CHROOT_SCRIPT, 'start'], $retval)) {
969998
logmsg(LOG_ERR, "chroot script exited with exitcode $retval");
970999
disable('judgehost', 'hostname', $myhost, "chroot script exited with exitcode $retval on $myhost");
9711000
continue;
@@ -1027,8 +1056,7 @@ function registerJudgehost(string $myhost): void
10271056

10281057
// Create directory where to test submissions
10291058
$workdirpath = JUDGEDIR . "/$myhost/endpoint-$endpointID";
1030-
system('mkdir -p ' . dj_escapeshellarg("$workdirpath/testcase"), $retval);
1031-
if ($retval !== 0) {
1059+
if (!run_command_safe(['mkdir', '-p', "$workdirpath/testcase"])) {
10321060
error("Could not create $workdirpath");
10331061
}
10341062
chmod("$workdirpath/testcase", 0700);
@@ -1106,8 +1134,7 @@ function cleanup_judging(string $workdir) : void
11061134

11071135
// destroy chroot environment
11081136
logmsg(LOG_INFO, " 🔓 Executing chroot script: '".CHROOT_SCRIPT." stop'");
1109-
system(LIBJUDGEDIR.'/'.CHROOT_SCRIPT.' stop', $retval);
1110-
if ($retval!==0) {
1137+
if (!run_command_safe([LIBJUDGEDIR.'/'.CHROOT_SCRIPT, 'stop'], $retval)) {
11111138
logmsg(LOG_ERR, "chroot script exited with exitcode $retval");
11121139
disable('judgehost', 'hostname', $myhost, "chroot script exited with exitcode $retval on $myhost");
11131140
// Just continue here: even though we might continue a current
@@ -1117,9 +1144,8 @@ function cleanup_judging(string $workdir) : void
11171144
}
11181145

11191146
// Evict all contents of the workdir from the kernel fs cache
1120-
system(LIBJUDGEDIR . '/evict ' . dj_escapeshellarg($workdir), $retval);
1121-
if ($retval!==0) {
1122-
warning("evict script exited with exitcode $retval");
1147+
if (!run_command_safe([LIBJUDGEDIR . '/evict', $workdir])) {
1148+
warning("evict script failed, continuing gracefully");
11231149
}
11241150
}
11251151

@@ -1128,7 +1154,7 @@ function compile(
11281154
string $workdir,
11291155
string $workdirpath,
11301156
array $compile_config,
1131-
string $cpuset_opt,
1157+
?string $daemonid,
11321158
int $output_storage_limit
11331159
): bool {
11341160
global $myhost, $EXITCODES;
@@ -1148,26 +1174,24 @@ function compile(
11481174
$args = 'hostname=' . urlencode($myhost);
11491175
foreach (['compiler', 'runner'] as $type) {
11501176
if (isset($version_verification[$type . '_version_command'])) {
1177+
if (file_exists($version_output_file)) {
1178+
unlink($version_output_file);
1179+
}
1180+
11511181
$vcscript_content = $version_verification[$type . '_version_command'];
11521182
$vcscript = tempnam(TMPDIR, 'version_check-');
11531183
file_put_contents($vcscript, $vcscript_content);
11541184
chmod($vcscript, 0755);
1155-
$version_cmd = LIBJUDGEDIR . "/version_check.sh " .
1156-
implode(' ', array_map('dj_escapeshellarg', [
1157-
$vcscript,
1158-
$workdir,
1159-
]));
11601185

1161-
if (file_exists($version_output_file)) {
1162-
unlink($version_output_file);
1163-
}
1164-
system($version_cmd, $retval);
1186+
run_command_safe([LIBJUDGEDIR . "/version_check.sh", $vcscript, $workdir], $retval);
1187+
11651188
$versions[$type] = trim(file_get_contents($version_output_file));
11661189
if ($retval !== 0) {
11671190
$versions[$type] =
11681191
"Getting $type version failed with exit code $retval\n"
11691192
. $versions[$type];
11701193
}
1194+
11711195
unlink($vcscript);
11721196
}
11731197
if (isset($versions[$type])) {
@@ -1245,16 +1269,14 @@ function compile(
12451269
}
12461270

12471271
// Compile the program.
1248-
$compile_cmd = LIBJUDGEDIR . "/compile.sh $cpuset_opt " .
1249-
implode(' ', array_map('dj_escapeshellarg', array_merge([
1250-
$execrunpath,
1251-
$workdir,
1252-
], $files)));
1253-
logmsg(LOG_DEBUG, "Compile command: ".$compile_cmd);
1254-
system($compile_cmd, $retval);
1255-
if ($retval!==0) {
1256-
warning("compile script exited with exitcode $retval");
1272+
$compile_command_parts = [LIBJUDGEDIR . '/compile.sh'];
1273+
if (isset($daemonid)) {
1274+
$compile_command_parts[] = '-n';
1275+
$compile_command_parts[] = $daemonid;
12571276
}
1277+
array_push($compile_command_parts, $execrunpath, $workdir, ...$files);
1278+
// Note that the $retval is handled further down after reading/writing metadata.
1279+
run_command_safe($compile_command_parts, $retval, log_nonzero_exitcode: false);
12581280

12591281
$compile_output = '';
12601282
if (is_readable($workdir . '/compile.out')) {
@@ -1355,7 +1377,7 @@ function judge(array $judgeTask): bool
13551377
}
13561378

13571379
$workdir = judging_directory($workdirpath, $judgeTask);
1358-
$compile_success = compile($judgeTask, $workdir, $workdirpath, $compile_config, $cpuset_opt, $output_storage_limit);
1380+
$compile_success = compile($judgeTask, $workdir, $workdirpath, $compile_config, $options['daemonid'] ?? null, $output_storage_limit);
13591381
if (!$compile_success) {
13601382
return false;
13611383
}
@@ -1457,37 +1479,39 @@ function judge(array $judgeTask): bool
14571479
// after the first (note that $passCnt starts at 1).
14581480
if ($passCnt > 1) {
14591481
$prevPassdir = $testcasedir . '/' . ($passCnt - 1) . '/feedback';
1460-
system('cp -R ' . dj_escapeshellarg($prevPassdir) . ' ' . dj_escapeshellarg($passdir . '/'));
1461-
system('rm ' . dj_escapeshellarg($passdir . '/feedback/nextpass.in'));
1482+
run_command_safe(['cp', '-R', $prevPassdir, $passdir . '/']);
1483+
run_command_safe(['rm', $passdir . '/feedback/nextpass.in']);
14621484
}
14631485

14641486
// Copy program with all possible additional files to testcase
14651487
// dir. Use hardlinks to preserve space with big executables.
14661488
$programdir = $passdir . '/execdir';
1467-
system('mkdir -p ' . dj_escapeshellarg($programdir), $retval);
1468-
if ($retval!==0) {
1489+
if (!run_command_safe(['mkdir', '-p', $programdir])) {
14691490
error("Could not create directory '$programdir'");
14701491
}
14711492

14721493
foreach (glob("$workdir/compile/*") as $compile_file) {
1473-
system('cp -PRl ' . dj_escapeshellarg($compile_file) . ' ' . dj_escapeshellarg($programdir), $retval);
1474-
if ($retval!==0) {
1494+
if (!run_command_safe(['cp', '-PRl', $compile_file, $programdir])) {
14751495
error("Could not copy program to '$programdir'");
14761496
}
14771497
}
14781498

14791499
$timelimit_str = implode(':', $timelimit['cpu']) . ',' . implode(':', $timelimit['wall']);
1480-
$test_run_cmd = LIBJUDGEDIR . "/testcase_run.sh $cpuset_opt " .
1481-
implode(' ', array_map('dj_escapeshellarg', [
1482-
$input,
1483-
$output,
1484-
$timelimit_str,
1485-
$passdir,
1486-
$run_runpath,
1487-
$compare_runpath,
1488-
$compare_config['compare_args']
1489-
]));
1490-
system($test_run_cmd, $retval);
1500+
$run_command_parts = [LIBJUDGEDIR . '/testcase_run.sh'];
1501+
if (isset($options['daemonid'])) {
1502+
$run_command_parts[] = '-n';
1503+
$run_command_parts[] = $options['daemonid'];
1504+
}
1505+
array_push($run_command_parts,
1506+
$input,
1507+
$output,
1508+
$timelimit_str,
1509+
$passdir,
1510+
$run_runpath,
1511+
$compare_runpath,
1512+
$compare_config['compare_args']
1513+
);
1514+
run_command_safe($run_command_parts, $retval, log_nonzero_exitcode: false);
14911515

14921516
// What does the exitcode mean?
14931517
if (!isset($EXITCODES[$retval])) {

0 commit comments

Comments
 (0)