Skip to content
This repository was archived by the owner on Jan 6, 2023. It is now read-only.

Commit 1d1a6c5

Browse files
author
Sebastien Boeuf
committed
exec: Return ERROR if the executed process could not start
The way hyperstart is designed, when we call into EXECCMD, we always receive an ACK on the ctl channel, even if the process could not start as expected. Because of this, the runtime does not raise any error and terminates returning 0. But this is wrong because it is expected to get the error not only from the shim, but from the runtime too. This bug has been found by running CRI-O bats. Specifically with the test case "ctr execsync failure" from ctr.bats. Signed-off-by: Sebastien Boeuf <[email protected]>
1 parent db2f540 commit 1d1a6c5

File tree

1 file changed

+50
-8
lines changed

1 file changed

+50
-8
lines changed

src/exec.c

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ struct stdio_config {
2929
};
3030

3131
static int hyper_release_exec(struct hyper_exec *);
32-
static void hyper_exec_process(struct hyper_exec *exec, struct stdio_config *io);
32+
static void hyper_exec_process(struct hyper_exec *exec, int sync_pipe_fd,
33+
struct stdio_config *io);
3334

3435
static int send_exec_finishing(uint64_t seq, int len, int code)
3536
{
@@ -475,7 +476,8 @@ static int hyper_setup_stdio_events(struct hyper_exec *exec, struct stdio_config
475476
return 0;
476477
}
477478

478-
static int hyper_do_exec_cmd(struct hyper_exec *exec, int pipe, struct stdio_config *io)
479+
static int hyper_do_exec_cmd(struct hyper_exec *exec, int pipe,
480+
int sync_pipe_fd, struct stdio_config *io)
479481
{
480482
struct hyper_container *c;
481483

@@ -517,15 +519,18 @@ static int hyper_do_exec_cmd(struct hyper_exec *exec, int pipe, struct stdio_con
517519
else
518520
unsetenv("TERM");
519521

520-
hyper_exec_process(exec, io);
522+
hyper_exec_process(exec, sync_pipe_fd, io);
521523

522524
out:
523525
_exit(125);
524526
}
525527

526528
// do the exec, no return
527-
static void hyper_exec_process(struct hyper_exec *exec, struct stdio_config *io)
529+
static void hyper_exec_process(struct hyper_exec *exec, int sync_pipe_fd,
530+
struct stdio_config *io)
528531
{
532+
int ret = 125;
533+
529534
if (sigprocmask(SIG_SETMASK, &orig_mask, NULL) < 0) {
530535
perror("sigprocmask restore mask failed");
531536
goto exit;
@@ -561,14 +566,21 @@ static void hyper_exec_process(struct hyper_exec *exec, struct stdio_config *io)
561566
/* the exit codes follow the `chroot` standard,
562567
see docker/docs/reference/run.md#exit-status */
563568
if (err == ENOENT)
564-
_exit(127);
569+
ret = 127;
565570
else if (err == EACCES)
566-
_exit(126);
571+
ret = 126;
572+
573+
if (sync_pipe_fd >= 0 &&
574+
write(sync_pipe_fd, &ret, sizeof(ret)) != sizeof(ret)) {
575+
perror("could not send error code through sync_pipe");
576+
}
577+
578+
_exit(ret);
567579
}
568580

569581
exit:
570582
fflush(NULL);
571-
_exit(125);
583+
_exit(ret);
572584
}
573585

574586
static void hyper_free_exec(struct hyper_exec *exec)
@@ -606,9 +618,24 @@ int hyper_exec_cmd(char *json, int length)
606618
return ret;
607619
}
608620

621+
int hyper_wait_process_start(int fd)
622+
{
623+
int ret = -1;
624+
625+
if (read(fd, &ret, sizeof(ret)) != sizeof(ret)) {
626+
// The other end has been closed because the process has been
627+
// properly started.
628+
return 0;
629+
}
630+
631+
fprintf(stderr, "error code %d received\n", ret);
632+
return ret;
633+
}
634+
609635
int hyper_run_process(struct hyper_exec *exec)
610636
{
611637
int pipe[2] = {-1, -1};
638+
int sync_pipe[2] = {-1, -1};
612639
int pid, ret = -1;
613640
uint32_t type;
614641
struct stdio_config io = {-1, -1,-1, -1,-1, -1};
@@ -634,15 +661,23 @@ int hyper_run_process(struct hyper_exec *exec)
634661
goto close_tty;
635662
}
636663

664+
if (pipe2(sync_pipe, O_CLOEXEC) < 0) {
665+
perror("create sync_pipe failed");
666+
goto close_tty;
667+
}
668+
637669
pid = fork();
638670
if (pid < 0) {
639671
perror("fork prerequisite process failed");
640672
goto close_tty;
641673
} else if (pid == 0) {
642-
hyper_do_exec_cmd(exec, pipe[1], &io);
674+
hyper_do_exec_cmd(exec, pipe[1], sync_pipe[1], &io);
643675
}
644676
fprintf(stdout, "prerequisite process pid %d\n", pid);
645677

678+
// Close sync_pipe fd from the parent.
679+
close_if_set(sync_pipe[1]);
680+
646681
if (hyper_get_type(pipe[0], &type) < 0 || (int)type < 0) {
647682
fprintf(stderr, "run process failed\n");
648683
goto close_tty;
@@ -653,6 +688,11 @@ int hyper_run_process(struct hyper_exec *exec)
653688
goto close_tty;
654689
}
655690

691+
if (hyper_wait_process_start(sync_pipe[0])) {
692+
fprintf(stderr, "process start failed\n");
693+
goto close_tty;
694+
}
695+
656696
exec->pid = type;
657697
list_add_tail(&exec->list, &exec->pod->exec_head);
658698
exec->ref++;
@@ -668,6 +708,8 @@ int hyper_run_process(struct hyper_exec *exec)
668708
if (io.stderrfd >= 0)
669709
close(io.stderrfd);
670710

711+
close(sync_pipe[0]);
712+
close(sync_pipe[1]);
671713
close(pipe[0]);
672714
close(pipe[1]);
673715
return ret;

0 commit comments

Comments
 (0)