[PATCH 2/4] fetch-pack: ignore SIGPIPE in sideband demuxer
- From
Jeff King <peff@peff.net>
- Date
- Feb 24, 2016, 07:44 UTC
- Message-ID
- <20160224074457.GB9481@sigill.intra.peff.net>
- In-Reply-To
- <20160224073603.GA9163@sigill.intra.peff.net>
If the other side feeds us a bogus pack, index-pack (or unpack-objects) may die early, before consuming all of its input. As a result, the sideband demuxer may get SIGPIPE (racily, depending on whether our data made it into the pipe buffer or not). If this happens and we are compiled with pthread support, it will take down the main thread, too.
This isn't the end of the world, as the main process will just die() anyway when it sees index-pack failed. But it does mean we don't get a chance to say "fatal: index-pack failed" or similar. And it also means that we racily fail t5504, as we sometimes die() and sometimes are killed by SIGPIPE.
So let's ignore SIGPIPE while demuxing the sideband. We are already careful to check the return value of write(), so we won't waste time writing to a broken pipe. The caller will notice the error return from the async thread, though in practice we don't even get that far, as we die() as soon as we see that index-pack failed.
The non-sideband case is already fine; we let index-pack read straight from the socket, so there is no SIGPIPE at all. Technically the non-threaded async case is also OK without this (the forked async process gets SIGPIPE), but it's not worth distinguishing from the threaded case here.
Signed-off-by: Jeff King <peff@peff.net> --- It's tempting to just ignore SIGPIPE in all async processes, as other sites may have similar problems (and we cannot depend on SIGPIPE taking down the main thread anyway, as async code may be implemented via fork()).
But that errs in the opposite direction. If an async process does not check the return value of write(), it may wastefully keep writing.
It would also mean implementing a pthread_sigmask() wrapper on Windows. I have no idea how feasible that would be.
fetch-pack.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/fetch-pack.c b/fetch-pack.c index 01e34b6..f96f6df 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -15,6 +15,7 @@ #include "version.h" #include "prio-queue.h" #include "sha1-array.h" +#include "sigchain.h" static int transfer_unpack_limit = -1; static int fetch_unpack_limit = -1; @@ -671,9 +672,12 @@ static int everything_local(struct fetch_pack_args *args, static int sideband_demux(int in, int out, void *data) { int *xd = data; + int ret; - int ret = recv_sideband("fetch-pack", xd[0], out); + sigchain_push(SIGPIPE, SIG_IGN); + ret = recv_sideband("fetch-pack", xd[0], out); close(out); + sigchain_pop(SIGPIPE); return ret; }
-- 2.7.2.645.g4e1306c