On Tue, Oct 22, 2013 at 5:25 PM, Jens Lindström <j...@opera.com> wrote:
> In a repository, I have a repeatable crash when pushing a ref to a
> remote. The cause seems very simple, and it's more unclear to me why
> this doesn't happen more often.
> The cause, as I understand it:
> git_transport_push() calls send_pack() which calls pack_objects()
> which calls start_command(), which closes the output file descriptor
> (known in start_command() as "cmd->out" and in git_transport_push() as
> "data->fd[1]".)
> git_transport_push(), immediately following the call to send_pack(),
> also closes the file descriptor. Adding error handling to this close()
> call shows that it fails with EBADF for a normal push (one that
> doesn't crash.)
> In my crashing scenario, the file descriptor has been reused between
> the first close() and the second incorrect close(), for opening a pack
> file. So the second close() closes the pack file, leaving a 'struct
> packed_git' object with a "dangling" pack_fd member. Later on, mmap()
> is called to map the contents of the pack file, but at this point the
> file descriptor has been reused yet again for a zero-length lock file,
> and so mmap() succeeds but returns no accessible memory, and we crash
> when accessing it.
> It would be trivial to fix this by simply removing the close() call in
> git_transport_push(), but I imagine this might cause a file descriptor
> leak in other cases instead.
> Thoughts on this?

set fd[1] = 0 after calling pack_objects() in send_pack() so that the
close() in git_transport_push() becomes no-op (with EBADF)? Another
option is dup() it first before passing to send_pack(), but I'm not
sure if it has any bad effects on start_command() and friends, or
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to