Thread (129 messages) 129 messages, 7 authors, 2017-04-25

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

From: Brandon Williams <hidden>
Date: 2017-04-11 17:18:06

On 04/11, Eric Wong wrote:
Brandon Williams [off-list ref] wrote:
quoted
On 04/11, Eric Wong wrote:
quoted
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.
Well if we move away from the (p) variant of exec, and do that
resolution ourselves, then we can use the existing
exists_in_PATH/locate_in_PATH logic (using strbufs!) and avoid needing
to do that string manipulation in plain C.  Of course we would then need
to adjust some of the errno handling (e.g. if it doesn't exist in the
path, we would then know that prior to forking so really no need to
handle that in the child anymore).

-- 
Brandon Williams
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help