Jeff King <peff@xxxxxxxx> writes:
> I share Junio's uneasiness with looping forever based on external input
> from the filesystem (even though you _should_ eventually win the race,
> that's not guaranteed, and of course a weird filesystem might confuse
> us).
Yeah, "a weird filesystem" would be a lot more plausible than a
determined and accurate attacker to break it. The only thing they
have to do is to yield EEXIST when failing link() for some other
reason.
> Could we put a stop-gap in it like:
>
> diff --git a/object-file.c b/object-file.c
> index 88432cc9c0..262a2f3df2 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -2038,6 +2038,7 @@ int finalize_object_file_flags(const char *tmpfile, const char *filename,
> enum finalize_object_file_flags flags)
> {
> int ret;
> + int retries = 0;
>
> retry:
> ret = 0;
> @@ -2080,8 +2081,11 @@ int finalize_object_file_flags(const char *tmpfile, const char *filename,
> }
> if (!(flags & FOF_SKIP_COLLISION_CHECK)) {
> ret = check_collision(tmpfile, filename);
> - if (ret == CHECK_COLLISION_DEST_VANISHED)
> + if (ret == CHECK_COLLISION_DEST_VANISHED) {
> + if (retries++ > 5)
> + return error(_("unable to write repeatedly vanishing file %s"), filename);
> goto retry;
> + }
> else if (ret)
> return -1;
> }
Sounds sensible.
> Otherwise, I think the logic looks good.
>
> -Peff
Thanks.
