system() with timeout

[Aaron Crane]

Abigail writes:
> On Thu, Apr 16, 2009 at 08:34:16PM +0100, David Cantrell wrote:
> >     eval {
> >         if(my $pid = fork()) { # parent
> >             local $SIG{ALRM} = sub {
> >                 kill 9, $pid; # quit RIGHT FUCKING NOW
> >                 die("Child process timed out\n");
> >             };
> >             alarm(5);
> >             waitpid($pid, 0);
> >             alarm(0);
> >         } else {
> >             exec(@blah);
> >         }
> >     };
> 
> I wonder whether that has a race condition. Could it be possible that
> the following happens:
> 
>   - child finishes.
>   - waitpid reaps the child.
>   - another process is started, with PID equal to $pid.
>   - 5 seconds are up and $SIG{ALRM} kicks in.

Yes, I agree.

I also think there's no way of closing the race window.  Whatever you
do to determine that the process $pid is what you expect it to be, you
can be rescheduled between that point and when you actually deliver
the SIGKILL to $pid.

If you can guarantee that the process to be timed out won't change its
process group, I think this would work:

- Parent process forks a child C and waits for it
- C creates a new process group with setpgrp(0, 0)
- C forks a grandchild G which will exec the desired process
- C sets up a SIGALRM handler that will SIGKILL the newly-created
  process group; if the SIGKILL gets delivered, it will kill C also
- C waits for G, and disables the alarm() when done
- C exits with the status it found by waiting for G

There's still a race there, but the consequence is now merely that C
exits with a SIGKILL status even though G exited successfully.  Since
that would have happened anyway if G had taken just a little longer to
complete, I consider that acceptable.

This approach also has the advantage of killing any children that G
itself has forked (although it might be preferable anyway for G to
setrlimit(RLIMIT_NPROC) with both soft and hard limits set to 0 before
executing the desired process).

It may be possible to deal with arbitrary processes that try to change
their process group by using ptrace(2) (or an equivalent mechanism on
your preferred OS).  This would count as a heroic measure, in my book.

In the absence of something like that, I can't think of a way to avoid
the race for an arbitrary process that might change its process group.
Fortunately, the window is normally going to be extremely small; it
relies on the pid being reused very quickly compared to the reschedule
interval.  In practice, I suspect it would be hard to trigger the race
in the absence of a malicious attacker who isn't subject to ulimits
and who's playing tricks like filling up the process table.  It may be
that the risk of encountering the race is sufficiently low as to make
it not worth worrying about.

There's another problem for an arbitrary process: if it's setuid, you
may not have permission to send it a signal, on some systems.  That
doesn't seem to be a problem given what David's trying to do here,
though.

-- 
Aaron Crane ** http://aaroncrane.co.uk/