Skip to content

faketime wrapper ignores pipe/fork failure and can exit 0 without running the command #551

Description

@yayong3

Summary

The faketime command (not the preload library) creates pipes and forks without checking the return value.

  • pipe(pfds) and pipe(keepalive_fds) are written as (void)(pipe(...) + 1). On failure the fd arrays are uninitialized, but the wrapper still close/dup/reads them and continues.
  • Both fork() sites use if (0 == (child_pid = fork())) { … } else { … }. fork() == -1 takes the parent branch. The final waitpid(child_pid, &ret, 0) then waits on -1; ret is uninitialized, and WIFSIGNALED / WEXITSTATUS are applied to it. Empirically the wrapper exits 0 and the user command never starts.

This is an error-handling bug. It shows up when the process is out of file descriptors (EMFILE) or cannot create another process (EAGAIN / pids.max). CI and containers hit both.

Present on 0.9.12 (bbb6c08), src/faketime.c.

Code

Date-parsing path (no -f):

/* src/faketime.c:212 */
int pfds[2];
(void) (pipe(pfds) + 1);
…
if (0 == (child_pid = fork())) {
  close(1);
  (void) (dup(pfds[1]) + 1);
  …
} else {
  char buf[256] = {0};
  close(pfds[1]);
  (void) (read(pfds[0], buf, 256) + 1);
  waitpid(child_pid, &ret, 0);
  …
}

Keepalive pipe and final exec:

/* :253, :397 */
int keepalive_fds[2];
(void) (pipe(keepalive_fds) + 1);
…
if (0 == (child_pid = fork())) {
  close(keepalive_fds[0]);
  execvp(argv[curr_opt], &argv[curr_opt]);
} else {
  int ret;                 /* not initialized */
  close(keepalive_fds[1]);
  waitpid(child_pid, &ret, 0);   /* child_pid == -1 → waitpid(-1, …) */exit(WEXITSTATUS(ret));
}

The + 1 expressions exist only to silence -Wunused-result; they do not handle failure.

Reproduce

Build the wrapper so it can find the library:

make -C src PREFIX="$(pwd)/src" LIBDIRNAME=

The hooks below force pipe/fork to fail. They show that the wrapper does not check the return value; they are not a claim that these syscalls fail on a healthy machine.

pipe ignored (faketime 'now' … creates the date pipe and the keepalive pipe):

#define _GNU_SOURCE
#include <errno.h>
#include <stdio.h>
#include <unistd.h>

int pipe(int pipefd[2])
{
  (void)pipefd;
  fprintf(stderr, "pipe() -> -1\n");
  errno = EMFILE;
  return -1;
}
gcc -O0 -fPIC -shared -o pipe_hook.so pipe_hook.c
LD_PRELOAD=./pipe_hook.so ./src/faketime 'now' /bin/true
# pipe() -> -1
# pipe() -> -1
# wrapper continues and exits 0

fork treated as parent (-f skips the date child, so the only fork is the user command):

#define _GNU_SOURCE
#include <errno.h>
#include <stdio.h>
#include <unistd.h>

pid_t fork(void)
{
  fprintf(stderr, "fork() -> -1\n");
  errno = EAGAIN;
  return -1;
}
gcc -O0 -fPIC -shared -o fork_hook.so fork_hook.c -ldl
rm -f /tmp/faketime-fork-marker
LD_PRELOAD=./fork_hook.so ./src/faketime -f '+0' \
  /bin/sh -c 'echo ran > /tmp/faketime-fork-marker'
# fork() -> -1
# wrapper exit 0; /tmp/faketime-fork-marker is absent

Suggested fix

Check pipe, fork, and waitpid. On failure, perror and exit(EXIT_FAILURE). Initialize ret. Do not treat fork() == -1 as the parent:

--- a/src/faketime.c
+++ b/src/faketime.c
@@ -210,10 +210,14 @@ int main (int argc, char **argv)
   if (!use_direct)
   {
     int pfds[2];
-    (void) (pipe(pfds) + 1);
+    if (pipe(pfds) == -1) {
+      perror("faketime: pipe");
+      exit(EXIT_FAILURE);
+    }
     int ret = EXIT_SUCCESS;
 
-    if (0 == (child_pid = fork()))
+    child_pid = fork();
+    if (child_pid == 0)
     {
       close(1);
-      (void) (dup(pfds[1]) + 1);
+      if (dup(pfds[1]) == -1) {
+        perror("faketime: dup");
+        exit(EXIT_FAILURE);
+      }
@@ -226,8 +230,16 @@ int main (int argc, char **argv)
       }
     }
+    else if (child_pid < 0)
+    {
+      perror("faketime: fork");
+      exit(EXIT_FAILURE);
+    }
     else
     {

Apply the same pattern to keepalive_fds and the final fork/waitpid (:253, :397). For the wait:

int ret = 0;
if (waitpid(child_pid, &ret, 0) == -1) {
  perror("faketime: waitpid");
  exit(EXIT_FAILURE);
}

Related (not this bug)

malloc of the LD_PRELOAD string at :387 is also unchecked. The allocation is a few dozen bytes; it is a separate robustness issue and does not need its own ticket.

Environment

  • wolfcw/libfaketime 0.9.12 (bbb6c08)
  • Linux x86_64, faketime built with PREFIX pointing at src/

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions