Subject: Re: [PATCH 6/5] run-command: avoid potential dangers in forked child

From: Eric Wong <[email-masked]>
Date: Tue, 11 Apr 2017 16:59:11 +0000

Brandon Williams <[email-masked]> wrote:
> On 04/11, Eric Wong wrote:
> > Hi Brandon, this series tickles an old itch of mine, so I
> > started working off of it.  I'm only somewhat concerned
> > with the path resolution in execvp(e) pontentially calling
> > malloc on some libcs; but I suppose that's a separate patch
> > for another time.
> > 
> > Only lightly-tested at the moment, but things seem to work...
> 
> Thanks Eric! I'll spend some time looking at this patch later today.  As
> for the path resolution in execvp(e), I guess we could completely avoid
> that if we did the path resolution ourselves, prior to forking, and then
> just use execv(e) since it shouldn't have any calls to malloc in them
> correct?

Yeah.  I spent some time looking at it last night, but emulating
the existing ENOENT / EACCESS / ENOTDIR mapping made my head
hurt.

And I'm not sure if I introduced any off-by-one errors in
exists_in_PATH when removing strbuf usage; string manipulation
in plain C scares me :x   Since memcpy/strcpy/getenv in there
are not specified as async-signal safe, they could
theoretically take locks and cause breakage inside a child.

I also wonder if there's a way to annotate internal functions as
async-signal safe (and thus vfork-child safe) besides sprinkling
comments in certain functions like xwrite.