Skip to content

Commit

Permalink
git-commit: fix double close(2) that can close a wrong file descriptor
Browse files Browse the repository at this point in the history
The codepath to prepare index files for the temporary and next
index file was closing file descriptor it obtained from the
lockfile API by hand, without letting the API know that the fd
should not be doubly closed.

This is not usually a problem (except it may get EBADFD) but if
we opened another fd for an entirely unrelated purpose (say, an
fd used to mmap a packfile) between the time we close the fd to
the index file and the time we commit or rollback the lockfile
(causing it to also try closing the recorded fd), the lockfile
API will close an incorrect file descriptor that is still used
for an entirely unrelated purpose.

There's four close(fd) calls in prepare_index() and they're all
incorrect.  The open fd's are cleaned up in rollback_index_files() and
shouldn't be closed manually.  The patch below gets rid of the extra
close() calls and should fix the problem.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Kristian Høgsberg authored and Junio C Hamano committed Jan 16, 2008
1 parent 1bc7c13 commit 4439751
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions builtin-commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
int fd = hold_locked_index(&index_lock, 1);
add_files_to_cache(0, also ? prefix : NULL, pathspec);
refresh_cache(REFRESH_QUIET);
if (write_cache(fd, active_cache, active_nr) || close(fd))
if (write_cache(fd, active_cache, active_nr))
die("unable to write new_index file");
commit_style = COMMIT_NORMAL;
return index_lock.filename;
Expand All @@ -256,7 +256,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
fd = hold_locked_index(&index_lock, 1);
refresh_cache(REFRESH_QUIET);
if (write_cache(fd, active_cache, active_nr) ||
close(fd) || commit_locked_index(&index_lock))
commit_locked_index(&index_lock))
die("unable to write new_index file");
commit_style = COMMIT_AS_IS;
return get_index_file();
Expand Down Expand Up @@ -298,7 +298,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
fd = hold_locked_index(&index_lock, 1);
add_remove_files(&partial);
refresh_cache(REFRESH_QUIET);
if (write_cache(fd, active_cache, active_nr) || close(fd))
if (write_cache(fd, active_cache, active_nr))
die("unable to write new_index file");

fd = hold_lock_file_for_update(&false_lock,
Expand All @@ -308,7 +308,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
add_remove_files(&partial);
refresh_cache(REFRESH_QUIET);

if (write_cache(fd, active_cache, active_nr) || close(fd))
if (write_cache(fd, active_cache, active_nr))
die("unable to write temporary index file");
return false_lock.filename;
}
Expand Down

0 comments on commit 4439751

Please sign in to comment.