Skip to content

Commit

Permalink
Fix: Could not fork too many open files
Browse files Browse the repository at this point in the history
Depending on the system when about to have 497 open forks with an file
descriptor open than a:

```
sd   main:WARNING:2022-09-21 08h52.59 utc:215235: Error : could not fork ! Error : Too many open files
```
occurs which results in an interrupted scan.

The reason for this are:
1. there was no handling in the parent process to close the fd
2. unfortunately plugins may exit instead of returning
3. the cache is not verified regulary if the process for the fd is
   running
4. the ipc cache was growing indefinitely

To solve the 1. case a signal SIGCHLD is send to the parent process when
the given function exit to trigger a ipc_close for that process.

To solve the 2. and 3. case there is a new function
`close_ipc_fd_on_unnoticed_exit` to close ipc when a process is not
running anymore. This function is called twice. Once before the fork
call is executed to close unneeded fd and on EMFILE || EAGAIN after
forking in a loop to retry forking after cleaning up unneeded
fd.

To solve the 4. case we check the cache upfront if there are closed ipc
cached and reuse those instead of a just appending new ipc.
  • Loading branch information
nichtsfrei committed Sep 23, 2022
1 parent aed0474 commit 354c42a
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 5 deletions.
4 changes: 2 additions & 2 deletions misc/ipc.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,8 @@ ipc_close (struct ipc_context *context)
switch (context->type)
{
case IPC_PIPE:
rc = ipc_pipe_close (context->context);
context->closed = 1;
if ((rc = ipc_pipe_close (context->context)) > -1)
context->closed = 1;
}
return rc;
}
Expand Down Expand Up @@ -212,6 +211,7 @@ ipc_exec_as_process (enum ipc_protocol type, struct ipc_exec_context exec_ctx)
ipc_destroy (pctx);
break;
}
kill (exec_ctx.parent, SIGCHLD);
exit (0);
}

Expand Down
1 change: 1 addition & 0 deletions misc/ipc.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ struct ipc_exec_context
void *func_arg; // argument for func
void *post_arg; // argument for post_func
void *shared_context; // context to be included in ipc_context
pid_t parent;
};

// ipc_process_func is a type for the function to be executed.
Expand Down
77 changes: 74 additions & 3 deletions src/processes.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,37 @@ clear_child ()
}
}
}
/**
* @brief iterates through ipcc and verify if a child is stopped or killed to
* free the file handler.
* @return the amount of freed file handler or -1 on ipcc not initialized
*/
static int
close_ipc_fd_on_unnoticed_exit ()
{
int freed = 0, i, status;
pid_t pid;
if (ipcc == NULL)
return -1;
g_debug ("%s: checking %d ipc.", __func__, ipcc->len);
for (i = 0; i < ipcc->len; i++)
{
if (ipcc->ctxs[i].closed)
{
continue;
}
pid = waitpid (ipcc->ctxs[i].pid, &status, WNOHANG);
if ((pid < 0)
|| ((pid == ipcc->ctxs[i].pid)
&& (WIFEXITED (status) || WIFSTOPPED (status)
|| WIFSIGNALED (status))))
{
freed++;
ipc_close (&ipcc->ctxs[i]);
}
}
return freed;
}

/**
* @brief Cleans the process list and frees memory. This will not terminate
Expand Down Expand Up @@ -213,6 +244,26 @@ post_fork_fun_call (struct ipc_context *ctx, void *args)
gvm_close_sentry ();
}

static void
reuse_or_add_context (struct ipc_context *ctx)
{
if (ipcc == NULL)
return;
for (int i = 0; i < ipcc->len; i++)
{
if (ipcc->ctxs[i].closed == 1)
{
ipcc->ctxs[i].context = ctx->context;
ipcc->ctxs[i].pid = ctx->pid;
ipcc->ctxs[i].relation = ctx->relation;
ipcc->ctxs[i].type = ctx->type;
ipcc->ctxs[i].closed = 0;
return;
}
}
ipc_add_context (ipcc, ctx);
}

/**
* @brief initializes a communication channels and calls a function with a new
* process
Expand All @@ -227,6 +278,7 @@ create_ipc_process (ipc_process_func func, void *args)
{
struct ipc_context *pctx = NULL;
struct ipc_exec_context ec;
pid_t child_pid;
// previously init call, we want to store the contexts without making
// assumptions about signal handlung
if (ipcc == NULL)
Expand All @@ -236,13 +288,32 @@ create_ipc_process (ipc_process_func func, void *args)
ec.post_func = (ipc_process_func) &post_fork_fun_call;
ec.func = (ipc_process_func) func;
ec.func_arg = args;
ec.parent = getpid ();
// check for exited processes and clean file descriptor
// we do it twice, before forking and when forking fails with EMFILE or EAGAIN
g_debug ("%s: closed %d fd due to premature exit.", __func__,
close_ipc_fd_on_unnoticed_exit ());
retry:
if ((pctx = ipc_exec_as_process (IPC_PIPE, ec)) == NULL)
{
g_warning ("Error : could not fork ! Error : %s", strerror (errno));
if (errno == EMFILE || errno == EAGAIN)
{
g_debug (
"%s: could not fork: %s (%d) retrying after trying to close fd.",
__func__, strerror (errno), errno);
g_debug ("%s: closed %d fd due to premature exit.", __func__,
close_ipc_fd_on_unnoticed_exit ());
goto retry;
}
g_warning ("%s: could not fork: %s (%d)", __func__, strerror (errno),
errno);
return FORKFAILED;
}
ipc_add_context (ipcc, pctx);
return pctx->pid;
reuse_or_add_context (pctx);
child_pid = pctx->pid;
// ipcc works uses copies of pctx therefore we free it
free (pctx);
return child_pid;
}

/**
Expand Down

0 comments on commit 354c42a

Please sign in to comment.