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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,4 @@ squashfs-root/

# build products
src/runtime/runtime
_codeql_detected_source_root
51 changes: 46 additions & 5 deletions src/runtime/runtime.c
Original file line number Diff line number Diff line change
Expand Up @@ -1846,11 +1846,52 @@ int main(int argc, char* argv[]) {
strcpy(filename, mount_dir);
strcat(filename, "/AppRun");

/* TODO: Find a way to get the exit status and/or output of this */
execv(filename, real_argv);
/* Error if we continue here */
perror("execv error");
exit(EXIT_EXECERROR);
/* Fork before exec to ensure we can close the keepalive pipe after AppRun exits */
pid_t apprun_pid = fork();
if (apprun_pid == -1) {
perror("fork error");
exit(EXIT_EXECERROR);
}

if (apprun_pid == 0) {
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

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

The child process inherits both ends of the keepalive pipe but doesn't use them. Both pipe ends should be closed before execv() to prevent the child from holding unnecessary file descriptors. Add close(keepalive_pipe[0]); and close(keepalive_pipe[1]); before the execv call on line 1858.

Suggested change
if (apprun_pid == 0) {
if (apprun_pid == 0) {
/* Child process - close keepalive pipe before exec */
close(keepalive_pipe[0]);
close(keepalive_pipe[1]);

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

This seems like an obvious oversight that was there before. I am just a little concerned that the pipe loses its usefulness when it is closed here already. (Yes, technically the parent process would keep its side open anyway).

This needs to be tested thoroughly. The nature of the bug requires a quite sophisticated setup, though, which is time consuming.

/* Child process - close keepalive pipe before exec */
close(keepalive_pipe[0]);

/* exec AppRun */
execv(filename, real_argv);
/* Error if we continue here */
perror("execv error");
exit(EXIT_EXECERROR);
} else {
/* Parent process - wait for AppRun to finish, then close pipe */
int status;
pid_t waited_pid;
do {
waited_pid = waitpid(apprun_pid, &status, 0);
} while (waited_pid == -1 && errno == EINTR);
if (waited_pid == -1) {
perror("waitpid error");
close(keepalive_pipe[0]);
free(real_argv);
exit(EXIT_EXECERROR);
}

/* Close the keepalive pipe after AppRun exits to terminate FUSE daemon */
close(keepalive_pipe[0]);

/* Free allocated memory before exit */
free(real_argv);

/* Exit with the same status as AppRun */
if (WIFEXITED(status)) {
exit(WEXITSTATUS(status));
} else if (WIFSIGNALED(status)) {
/* Child was killed by a signal, exit with 128 + signal number */
exit(128 + WTERMSIG(status));
} else {
exit(EXIT_EXECERROR);
Comment on lines +1886 to +1892
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

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

The memory allocated for real_argv on line 1807 is never freed before these exit calls, causing a memory leak. While the process is exiting anyway, it's better practice to free(real_argv) before the exit calls or restructure to ensure cleanup happens.

Copilot uses AI. Check for mistakes.
}
}
}

return 0;
Expand Down