Re: [PATCH v4 1/8] finalize_object_file(): check for name collision before renaming
From: Junio C Hamano <hidden>
Date: 2024-09-25 17:02:50
Taylor Blau [off-list ref] writes:
... But in practice it is expanding the definition of "what is already on disk" to be the point that the function is called.
Yeah, it is a reasonable argument for this additional protection. It does not make things worse. All it takes is for the attacker to come a bit earlier to defeat the link/unlink dance, so doing it "the right way" does not make it fundamentally safer. I hope all TOCTOU races can be explained away this way ;-).
quoted hunk
Co-authored-by: Jeff King [off-list ref] Signed-off-by: Jeff King <redacted> Signed-off-by: Taylor Blau <redacted> --- object-file.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)diff --git a/object-file.c b/object-file.c index 968da27cd41..38407f468a9 100644 --- a/object-file.c +++ b/object-file.c@@ -1937,6 +1937,7 @@ static void write_object_file_prepare_literally(const struct git_hash_algo *algo */ int finalize_object_file(const char *tmpfile, const char *filename) { + struct stat st; int ret = 0; if (object_creation_mode == OBJECT_CREATION_USES_RENAMES)@@ -1957,9 +1958,12 @@ int finalize_object_file(const char *tmpfile, const char *filename) */ if (ret && ret != EEXIST) { try_rename: - if (!rename(tmpfile, filename)) + if (!stat(filename, &st)) + ret = EEXIST; + else if (!rename(tmpfile, filename)) goto out; - ret = errno; + else + ret = errno; } unlink_or_warn(tmpfile); if (ret) {