Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 32 additions & 13 deletions judge/chroot-startstop.sh.in
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ if [ "$(uname -m)" = "x86_64" ]; then
fi

# Location of the pre-built chroot tree and where to bind mount from:
CHROOTORIGINAL="@judgehost_chrootdir@"
CHROOTDIR="@judgehost_chrootdir@"

dj_umount() {
set +e
Expand All @@ -54,22 +54,39 @@ dj_umount() {
set -e
}

CHROOTDIR_OPT=""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like CHROOTDIR_OPT might be unused?

while getopts "c:" opt; do
case $opt in
c)
CHROOTDIR="${CHROOTDIR//domjudge/$OPTARG}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about handling $OPTARG relative to $(dirname "$CHROOTDIR") (or checking if it's an absolute path) rather than doing string substitution on domjudge?

One catch with ${CHROOTDIR//domjudge/$OPTARG} is that if a language has no custom chroot set it replaces domjudge with default and looks for /chroot/default instead of the original /chroot/domjudge. It would also be tricky if someone configured --with-judgehost-chrootdir with a path that doesn't include the literal word domjudge.

Perhaps something like:

if [ -z "$OPTARG" ] || [ "$OPTARG" = "default" ]; then                                                                                                               
        # keep default CHROOTDIR                                                                                                                                         
        :                                                                                                                                                                
elif [[ "$OPTARG" = /* ]]; then                                                                                                                                      
        CHROOTDIR="$OPTARG"                                                                                                                                              
else                                                                                                                                                                 
        CHROOTDIR="$(dirname "$CHROOTDIR")/$OPTARG"                                                                                                                      
fi                                                                                                                                                                   

What are your thoughts?

;;
:)
echo "Option -$OPTARG requires an argument." >&2
;;
*)
echo "Invalid option specified." >&2
exit 1
;;
esac
done
shift $((OPTIND-1))

case "$1" in
check)
if [ ! -d "$CHROOTORIGINAL" ]; then
>&2 echo "chroot dir '$CHROOTORIGINAL' does not exist, run dj_make_chroot"
if [ ! -d "$CHROOTDIR" ]; then
>&2 echo "chroot dir '$CHROOTDIR' does not exist, run dj_make_chroot"
exit 2
fi
for i in $SUBDIRMOUNTS ; do
if [ ! -e "$CHROOTORIGINAL/$i" ]; then
>&2 echo "chroot subdir '$CHROOTORIGINAL/$i' not found, rerun dj_make_chroot"
if [ ! -e "$CHROOTDIR/$i" ]; then
>&2 echo "chroot subdir '$CHROOTDIR/$i' not found, rerun dj_make_chroot"
exit 2
fi
done
# This directory is removed at the very end of the debootstrap run, if it's
# still present, building the chroot did not complete.
if [ -d "$CHROOTORIGINAL/debootstrap" ]; then
>&2 echo "chroot dir '$CHROOTORIGINAL' incomplete, rerun dj_make_chroot"
if [ -d "$CHROOTDIR/debootstrap" ]; then
>&2 echo "chroot dir '$CHROOTDIR' incomplete, rerun dj_make_chroot"
exit 2
fi
# Check that sudo works
Expand All @@ -94,11 +111,11 @@ case "$1" in

# Some dirs may be links to others, e.g. /lib64 -> /lib.
# Preserve those; bind mount the others.
if [ -L "$CHROOTORIGINAL/$i" ]; then
ln -s "$(readlink "$CHROOTORIGINAL/$i")" "$i"
elif [ -d "$CHROOTORIGINAL/$i" ]; then
if [ -L "$CHROOTDIR/$i" ]; then
ln -s "$(readlink "$CHROOTDIR/$i")" "$i"
elif [ -d "$CHROOTDIR/$i" ]; then
mkdir -p $i
sudo -n mount --bind "$CHROOTORIGINAL/$i" "$i" < /dev/null
sudo -n mount --bind "$CHROOTDIR/$i" "$i" < /dev/null
# Mount read-only for extra security. Note that this
# must be executed separately from the bind mount.
sudo -n mount -o remount,ro,bind "$PWD/$i" < /dev/null
Expand All @@ -119,9 +136,9 @@ case "$1" in
rmdir dev || true

for i in $SUBDIRMOUNTS ; do

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In stop, checking $CHROOTDIR/$i to decide whether to unmount or remove symlinks could be slightly fragile if stop is called without -c or with a different chroot than the one currently mounted.

Could we check the target in $PWD directly instead? That way stop wouldn't need to know which chroot was originally mounted to clean up properly.

if [ -L "$CHROOTORIGINAL/$i" ]; then
if [ -L "$CHROOTDIR/$i" ]; then
rm -f "$i"
elif [ -d "$CHROOTORIGINAL/$i" ]; then
elif [ -d "$CHROOTDIR/$i" ]; then
dj_umount "$PWD/$i"
fi
done
Expand All @@ -133,6 +150,8 @@ case "$1" in

*)
echo "Unknown argument '$1' given."
echo "Valid arguments: check, start & stop."
echo "Valid options: -c <custom_chroot>."
exit 1
esac

Expand Down
121 changes: 118 additions & 3 deletions judge/judgedaemon.main.php
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,11 @@ public function __construct(
* team_message?: string, score?: string
* }
* @phpstan-type RunConfig array{time_limit: float, memory_limit: int, output_limit: int,
* process_limit: int, entry_point: ?string, pass_limit: int, hash: string, overshoot: int
* process_limit: int, entry_point: ?string, pass_limit: int, hash: string, overshoot: int, chroot?: string
* }
* @phpstan-type CompareConfig array{script_timelimit: int, script_memory_limit: int,
* script_filesize_limit: int, compare_args: string, combined_run_compare: bool,
* hash: string, is_scoring_problem: bool
* hash: string, is_scoring_problem: bool, chroot?: string
* }
* @phpstan-import-type MetaData_Compare from CompareMetaData
* @phpstan-import-type MetaData_Program from ProgramMetaData
Expand Down Expand Up @@ -215,6 +215,11 @@ class JudgeDaemon
/** @var array<string, string[]> */
private array $langexts = [];

/** @var string[] */
private array $chroots_checked = [];

private string $chroot_current = 'default';

/** @var ?resource */
private $lockfile;
/** @var array<int, string> */
Expand Down Expand Up @@ -412,6 +417,8 @@ private function initialize(): void
logmsg(LOG_INFO, "🔏 Executing chroot script: '" . self::CHROOT_SCRIPT . " check'");
if (!$this->runCommandSafe([LIBJUDGEDIR . '/' . self::CHROOT_SCRIPT, 'check'])) {
error("chroot validation check failed");
} else {
$this->chroots_checked = ['default'];
}

$this->registerJudgehost();
Expand Down Expand Up @@ -608,6 +615,12 @@ private function handleJudgingTask(array $row, ?string &$lastWorkdir, string $wo
if ($lastWorkdir !== $workdir) {
// create chroot environment
logmsg(LOG_INFO, " 🔒 Executing chroot script: '" . self::CHROOT_SCRIPT . " start'");
// Reset the value to the default chroot, this should already be set by startup or cleanup at the end.
if ($this->chroot_current !== 'default') {
logmsg(LOG_INFO, "Found 'chroot_current' to not be the default, either setup or cleanup failed.");
$this->chroot_current = 'default';
}
logmsg(LOG_DEBUG, "Currently working from '" . getcwd() . "' directory.");
if (!$this->runCommandSafe([LIBJUDGEDIR . '/' . self::CHROOT_SCRIPT, 'start'], $retval)) {
logmsg(LOG_ERR, "chroot script exited with exitcode $retval");
$this->disable('judgehost', 'hostname', $this->myhost, "chroot script exited with exitcode $retval on $this->myhost");
Expand Down Expand Up @@ -1570,7 +1583,7 @@ private function cleanupJudging(string $workdir): void

/**
* @param JudgeTask $judgeTask
* @param array{script_timelimit: int, script_memory_limit: int, language_extensions: array<string>, filter_compiler_files: bool, hash: string} $compile_config
* @param array{script_timelimit: int, script_memory_limit: int, language_extensions: array<string>, filter_compiler_files: bool, hash: string, chroot?: string} $compile_config
*/
private function compile(
array $judgeTask,
Expand All @@ -1585,6 +1598,41 @@ private function compile(
return true;
}

$chroot_compile = $compile_config['chroot'] ?? 'default';
if ($chroot_compile !== $this->chroot_current) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should extract all of this duplication into some helper logic.

logmsg(LOG_INFO, " 🔏 Submission should be done in different chroot '" . $chroot_compile . "', leaving chroot '" . $this->chroot_current . "'");
logmsg(LOG_INFO, " 🔓 Executing chroot script: '" . self::CHROOT_SCRIPT . " stop'");
sleep(1);
if (!$this->runCommandSafe([LIBJUDGEDIR . '/' . self::CHROOT_SCRIPT, 'stop'], $retval)) {
logmsg(LOG_ERR, "chroot script exited with exitcode $retval");
$this->disable('judgehost', 'hostname', $this->myhost, "chroot script exited with exitcode $retval on $this->myhost");
return false;
// Leaving this here for the review, I think we can decide to leave here as we know we didn´t compile yet and we failed.
// rm: Just continue here: even though we might continue a current
// rm: compile/test-run cycle, we don't know whether we're in one here,
// rm: and worst case, the chroot script will fail the next time when
// rm: starting.
}
sleep(1);
if (!in_array($chroot_compile, $this->chroots_checked)) {
logmsg(LOG_INFO, " 🔏 Executing chroot script: '" . self::CHROOT_SCRIPT . " -c " . $chroot_compile . " check'");
if (!$this->runCommandSafe([LIBJUDGEDIR . '/' . self::CHROOT_SCRIPT, '-c', $chroot_compile, 'check'])) {
error("chroot validation check failed");
} else {
$this->chroots_checked[] = $chroot_compile;
}
}
sleep(1);
logmsg(LOG_INFO, " 🔒 Executing chroot script: '" . self::CHROOT_SCRIPT . " -c " . $chroot_compile . " start'");
if (!$this->runCommandSafe([LIBJUDGEDIR . '/' . self::CHROOT_SCRIPT, '-c', $chroot_compile, 'start'], $retval)) {
logmsg(LOG_ERR, "chroot script exited with exitcode $retval");
$this->disable('judgehost', 'hostname', $this->myhost, "chroot script exited with exitcode $retval on $this->myhost");
return false;
}
sleep(1);
$this->chroot_current = $chroot_compile;
}

// Verify compile and runner versions.
$judgeTaskId = $judgeTask['judgetaskid'];
$version_verification = dj_json_decode($this->request('judgehosts/get_version_commands/' . $judgeTaskId, 'GET'));
Expand Down Expand Up @@ -1972,6 +2020,39 @@ private function testcaseRunInternal(
return Verdict::INTERNAL_ERROR;
}

$chroot_run = $run_config['chroot'] ?? 'default';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we missing something like this here?

$this->chroot_current = $chroot_run;

if ($chroot_run !== $this->chroot_current) {
logmsg(LOG_INFO, " 🔏 Submission should be done in different chroot '" . $chroot_run . "', leaving chroot '" . $this->chroot_current . "'");
logmsg(LOG_INFO, " 🔓 Executing chroot script: '" . self::CHROOT_SCRIPT . " stop'");
sleep(1);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all the sleep(1)s intentional or debugging? We optimized them out years ago and this regresses the state.

if (!$this->runCommandSafe([LIBJUDGEDIR . '/' . self::CHROOT_SCRIPT, 'stop'], $retval)) {
logmsg(LOG_ERR, "chroot script exited with exitcode $retval");
$this->disable('judgehost', 'hostname', $this->myhost, "chroot script exited with exitcode $retval on $this->myhost");
return Verdict::INTERNAL_ERROR;
// Leaving this here for the review, I think we can decide to leave here as we know we didn´t compile yet and we failed.
// rm: Just continue here: even though we might continue a current
// rm: compile/test-run cycle, we don't know whether we're in one here,
// rm: and worst case, the chroot script will fail the next time when
// rm: starting.
}
sleep(1);
if (!in_array($chroot_run, $this->chroots_checked)) {
logmsg(LOG_INFO, " 🔏 Executing chroot script: '" . self::CHROOT_SCRIPT . " -c " . $chroot_run . " check'");
if (!$this->runCommandSafe([LIBJUDGEDIR . '/' . self::CHROOT_SCRIPT, '-c', $chroot_run, 'check'])) {
error("chroot validation check failed");
} else {
$this->chroots_checked[] = $chroot_run;
}
}
sleep(1);
logmsg(LOG_INFO, " 🔒 Executing chroot script: '" . self::CHROOT_SCRIPT . " -c " . $chroot_run . " start'");
if (!$this->runCommandSafe([LIBJUDGEDIR . '/' . self::CHROOT_SCRIPT, '-c', $chroot_run, 'start'], $retval)) {
logmsg(LOG_ERR, "chroot script exited with exitcode $retval");
$this->disable('judgehost', 'hostname', $this->myhost, "chroot script exited with exitcode $retval on $this->myhost");
return Verdict::INTERNAL_ERROR;
}
sleep(1);
}
$realWorkdir = realpath($passdir);
$prefix = '/' . basename(dirname($realWorkdir)) . '/' . basename($realWorkdir);
if (!chdir($realWorkdir)) {
Expand Down Expand Up @@ -2183,6 +2264,39 @@ private function testcaseRunInternal(
$orig_compare_args = str_getcsv($compare_args, separator: ' ', escape: '');
}

$chroot_compare = $run_config['chroot'] ?? 'default';
if ($chroot_compare !== $this->chroot_current) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point in execution, the working directory has already been changed to $realWorkdir if I am not mistaken so running chroot-startstop.sh here would try to mount inside the pass subdirectory rather than the judging workdir.

logmsg(LOG_INFO, " 🔏 Submission comparisan should be done in different chroot '" . $chroot_compare . "', leaving chroot '" . $this->chroot_current . "'");
logmsg(LOG_INFO, " 🔓 Executing chroot script: '" . self::CHROOT_SCRIPT . " stop'");
sleep(1);
if (!$this->runCommandSafe([LIBJUDGEDIR . '/' . self::CHROOT_SCRIPT, 'stop'], $retval)) {
logmsg(LOG_ERR, "chroot script exited with exitcode $retval");
$this->disable('judgehost', 'hostname', $this->myhost, "chroot script exited with exitcode $retval on $this->myhost");
return Verdict::INTERNAL_ERROR;
// Leaving this here for the review, I think we can decide to leave here as we know we didn´t compile yet and we failed.
// rm: Just continue here: even though we might continue a current
// rm: compile/test-run cycle, we don't know whether we're in one here,
// rm: and worst case, the chroot script will fail the next time when
// rm: starting.
}
sleep(1);
if (!in_array($chroot_compare, $this->chroots_checked)) {
logmsg(LOG_INFO, " 🔏 Executing chroot script: '" . self::CHROOT_SCRIPT . " -c " . $chroot_compare . " check'");
if (!$this->runCommandSafe([LIBJUDGEDIR . '/' . self::CHROOT_SCRIPT, '-c', $chroot_compare, 'check'])) {
error("chroot validation check failed");
} else {
$this->chroots_checked[] = $chroot_compare;
}
}
sleep(1);
logmsg(LOG_INFO, " 🔒 Executing chroot script: '" . self::CHROOT_SCRIPT . " -c " . $chroot_compare . " start'");
if (!$this->runCommandSafe([LIBJUDGEDIR . '/' . self::CHROOT_SCRIPT, '-c', $chroot_compare, 'start'], $retval)) {
logmsg(LOG_ERR, "chroot script exited with exitcode $retval");
$this->disable('judgehost', 'hostname', $this->myhost, "chroot script exited with exitcode $retval on $this->myhost");
return Verdict::INTERNAL_ERROR;
}
sleep(1);
}
$compare_args = array_merge(
$gainroot,
[BINDIR . "/runguard"],
Expand Down Expand Up @@ -2405,6 +2519,7 @@ private function runTestcase(
// compare script
$compare_runpath = '';
} else {
// Debug, are we actually in the chroot here? Test by breaking the underlying fetchExectuable code?
[$compare_runpath, $error] = $this->fetchExecutable(
$workdirpath,
'compare',
Expand Down
34 changes: 34 additions & 0 deletions webapp/migrations/Version20260602175403.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

declare(strict_types=1);

namespace DoctrineMigrations;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

/**
* Auto-generated Migration: Please modify to your needs!
*/
final class Version20260602175403 extends AbstractMigration
{
public function getDescription(): string
{
return 'Install extra optional chroot directory option for a language.';
}

public function up(Schema $schema): void
{
$this->addSql('ALTER TABLE language ADD chroot_directory VARCHAR(32) DEFAULT NULL COMMENT \'Custom chroot for executable\'');
}

public function down(Schema $schema): void
{
$this->addSql('ALTER TABLE language DROP chroot_directory');
}

public function isTransactional(): bool
{
return false;
}
}
14 changes: 14 additions & 0 deletions webapp/src/Entity/Language.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,9 @@ class Language extends BaseApiEntity implements
#[Serializer\Exclude]
private Collection $problems;

#[ORM\Column(length: 32, nullable: true, options: ['comment' => 'Custom chroot for executable'])]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, we should probably add Serializer\Exclude.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps also #[Assert\Regex] constraint (e.g. ^[a-zA-Z0-9_-]+$) to ensure only clean directory names?

private ?string $chroot_directory;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: chrootDirectory


/**
* @param Collection<int, Version> $versions
*/
Expand Down Expand Up @@ -471,4 +474,15 @@ public function getProblems(): Collection
{
return $this->problems;
}

public function setChrootDirectory(?string $chrootDirectory): self
{
$this->chroot_directory = $chrootDirectory;
return $this;
}

public function getChrootDirectory(): ?string
{
return $this->chroot_directory;
}
}
3 changes: 3 additions & 0 deletions webapp/src/Form/Type/LanguageType.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
$builder->add('entryPointDescription', TextType::class, [
'required' => false,
]);
$builder->add('chrootDirectory', TextType::class, [
'required' => false,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a descriptive help message here.

]);
$builder->add('allowSubmit', CheckboxType::class, [
'required' => false,
'attr' => self::TOGGLE_ATTRS,
Expand Down
Loading
Loading