Subject: Re: [PATCH 04/10] packed-backend: add "packed-refs" header consistency check

From: shejialuo <[email-masked]>
Date: Mon, 17 Feb 2025 21:16:11 +0800

On Thu, Jan 16, 2025 at 02:57:37PM +0100, Patrick Steinhardt wrote:

[snip]

> > @@ -1779,7 +1867,24 @@ static int packed_fsck(struct ref_store *ref_store,
> >  		goto cleanup;
> >  	}
> >  
> > +	if (strbuf_read_file(&packed_ref_content, refs->path, 0) < 0) {
> > +		/*
> > +		 * Although we have checked that the file exists, there is a possibility
> > +		 * that it has been removed between the lstat() and the read attempt by
> > +		 * another process. In that case, we should not report an error.
> > +		 */
> > +		if (errno == ENOENT)
> > +			goto cleanup;
> 
> Unlikely, but good to guard us against that condition regardless. It's
> still not entirely race-free though because the file could meanwhile
> have changed into a symlink, and we wouldn't notice now. We could fix
> that by using open(O_NOFOLLOW), fstat the returne file descriptor and
> then use `strbuf_read()` to slurp in the file.
> 

I have been looking back to the original discussion. I will follow this
advice which eventually avoids the race.

Thanks,
Jialuo