Skip to content

init: Fix args processing #599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
slp merged 3 commits into from
Mar 27, 2026
Merged

init: Fix args processing #599

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0186928
init: Handle failed memory allocations in argv processing
mz-pdm Mar 25, 2026
f3e4f89
init: Fix out of argv array bounds accesses
mz-pdm Mar 23, 2026
007e40b
init: Remove MAX_ARGS limit
mz-pdm Mar 23, 2026
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
43 changes: 31 additions & 12 deletions init/init.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
#define KRUN_FOOTER_LEN 12
#define CMDLINE_SECRET_PATH "/sfs/secrets/coco/cmdline"
#define CONFIG_FILE_PATH "/.krun_config.json"
#define MAX_ARGS 32
#define MAX_PASS_SIZE 512
#define MAX_TOKENS 16384

Expand Down Expand Up @@ -623,32 +622,42 @@ static char **config_parse_args(char *data, jsmntok_t *token)
char *arg, *value;
char **argv;
int len;
int i, j;
int i;
const int n_args = token->size;

argv = malloc(MAX_ARGS * sizeof(char *));
j = 0;
argv = malloc((n_args + 1) * sizeof(char *));
if (!argv) {
perror("malloc(config_parse_args)");
return NULL;
}

Comment thread
mz-pdm marked this conversation as resolved.
for (i = 0; i < token->size; i++) {
for (i = 0; i < n_args; i++) {
targ = &token[i + 1];

value = data + targ->start;
len = targ->end - targ->start;

arg = malloc(len + 1);
if (!arg) {
perror("malloc(config_parse_args arg)");
while (--i >= 0)
free(argv[i]);
free(argv);
return NULL;
Comment thread
mz-pdm marked this conversation as resolved.
}
memcpy(arg, value, len);
arg[len] = '\0';

unescape_string(arg, len);

argv[j] = arg;
j++;
argv[i] = arg;
}

if (j == 0) {
if (i == 0) {
free(argv);
argv = NULL;
} else {
argv[j] = NULL;
argv[i] = NULL;
}

return argv;
Expand Down Expand Up @@ -692,14 +701,24 @@ char **concat_entrypoint_argv(char **entrypoint, char **config_argv)
{
char **argv;
int i, j;
int n_args = 0;

for (i = 0; entrypoint[i]; i++)
n_args++;
for (j = 0; config_argv[j]; j++)
n_args++;

argv = malloc(MAX_ARGS * sizeof(char *));
argv = malloc((n_args + 1) * sizeof(char *));
if (!argv) {
perror("malloc(concat_entrypoint_argv)");
return NULL;
}

for (i = 0; i < MAX_ARGS && entrypoint[i]; i++) {
for (i = 0; entrypoint[i]; i++) {
argv[i] = entrypoint[i];
}

for (j = 0; j < MAX_ARGS && config_argv[j]; i++, j++) {
for (j = 0; config_argv[j]; i++, j++) {
argv[i] = config_argv[j];
Comment thread
mz-pdm marked this conversation as resolved.
}
Comment on lines +706 to 723
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 25, 2026

Choose a reason for hiding this comment

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

critical

There are two issues in this function:

  1. Critical: The return value of malloc on line 700 is not checked. If malloc fails, it will cause a segmentation fault on line 703.
  2. Medium: The current implementation iterates over entrypoint and config_argv arrays twice (once for counting, once for copying). This is inefficient.

A more robust and efficient implementation would check for malloc failure and use memcpy for copying after determining the lengths of the arrays. Here is a suggested refactoring of the function body:

    char **argv;
    int entrypoint_len = 0;
    int config_argv_len = 0;

    while (entrypoint[entrypoint_len]) {
        entrypoint_len++;
    }
    while (config_argv[config_argv_len]) {
        config_argv_len++;
    }

    int n_args = entrypoint_len + config_argv_len;
    argv = malloc((n_args + 1) * sizeof(char *));
    if (!argv) {
        perror("malloc");
        return NULL;
    }

    memcpy(argv, entrypoint, entrypoint_len * sizeof(char *));
    memcpy(argv + entrypoint_len, config_argv, config_argv_len * sizeof(char *));
    argv[n_args] = NULL;

    return argv;

Note that i and j would need to be removed from the variable declarations if you use this approach.

Copy link
Copy Markdown
Collaborator Author

@mz-pdm mz-pdm Mar 25, 2026

Choose a reason for hiding this comment

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

This may not be a bad idea but it introduces unnecessary changes to the problem being fixed and I consider the original code a bit better readable. As for malloc() return value, this is handled in the newly added commit.


Expand Down
Loading