Re: [PATCH 0/9] refs: stop using `chdir_notify_reparent()`

[Patrick Steinhardt <ps@pks.im>]

On Sat, Jun 13, 2026 at 10:00:24AM -0400, Jeff King wrote:
> On Fri, Jun 12, 2026 at 08:18:16AM +0200, Patrick Steinhardt wrote:
> 
> > > If we move to a world of all absolute paths where chdir-notify is not
> > > necessary, will we lose that optimization?
> > 
> > Probably. Unfortunately, the commit doesn't have any repeatable
> > benchmarks in there, so it's hard to say whether we could still
> > reproduce those issues or not.
> 
> Here's an easy-ish reproduction specific to the ref code:
> 
>   rm -rf a/
>   dir=$(perl -e 'print "a/" x 1024')
>   mkdir -p $dir &&
>   cd $dir &&
>   git init &&
>   git commit --allow-empty -m foo &&
>   seq -f 'create refs/heads/foo%05g HEAD' 10000 |
>   git update-ref --stdin &&
>   time git show-ref
> 
> Before your series, I get timings like this:
> 
>   real	0m0.078s
>   user	0m0.020s
>   sys	0m0.057s
> 
> After, I get:
> 
>   real	0m0.876s
>   user	0m0.004s
>   sys	0m0.872s
> 
> So it really is measurable (and I did not expect the effect to be nearly
> so large). Unsurprisingly the extra CPU goes to system time.

This is indeed surprisingly bad.

> But obviously that case is quite silly. It's an absurdly deep hierarchy,
> and 10,000 loose refs is a lot. Just running "git pack-refs --all"
> brings the before/after to roughly the same timings (around 40ms --
> faster even than the before timing).
> 
> So it _can_ matter, but I think ultimately the better direction is
> probably "make fewer syscalls". Which we do via packfiles, and via
> packed-refs, and eventually via reftables, all of which put more data
> into a single file.
> 
> I offer the script above more as food for thought, and not necessarily
> an argument against your series.

Hum, yeah. I'm a bit hesitant to just wave your findings away. I mean I
agree with you that it's unlikely to really matter in practice. But you
never really know, and I'm not sure that I consider dropping the chdir
infra important enough to knowingly take that hit.

I definitely think that we should merge the remainder of this series
though, as these patches simplify "setup.c" and fix a couple of memory
leaks. But maybe we drop the last patch for now and...

> > Ideally, we'd have the best of both worlds: absolute paths everywhere
> > without the performance hit. A while back I had a discussion with
> > Torvalds on the securiy mailing list around this issue, and ultimately
> > the conclusion was that the best way forward would be to use openat(3p).
> > 
> > This wouldn't only allow us to optimize cases like this, but it also has
> > the added benefit that we're much less prone to TOCTOU-style issues and
> > we might even be able to use flags like O_BENEATH. So it would basically
> > be win-win. The only problem is of course that Windows doesn't have
> > openat(3p), so we'd have to emulate it, and that's where I always lost
> > the desire to do this.
> > 
> > When waking up this morning though I had the thought that we shouldn't
> > try to emulate openat(3p) directly, but instead create a higher-level
> > interface.
> > [...]
> 
> Yeah, I think given a decent interface it might not be so bad. It would
> mean code thinking about filesystem syscalls in a different way, but if
> done subsystem-by-subsystem it might be OK to do incrementally. Much of
> the code that would want to switch to this is using repo_git_path() or
> similar already (and getting rid of those remaining static-buffer
> functions would be a nice bonus).
> 
> I do wonder if your series here to move to absolute paths makes the
> TOCTOU situation a little worse. With a relative path, once we are
> "inside" the repo then we are only susceptible to changes within it.
> Whereas with an absolute path, if one of the intermediate paths changes
> from under us, there may be confusion.
> 
> Without thinking on it too hard, though, I'd guess if any such case is a
> security problem, it already was during the "open" part (because it
> implies that the attacker controls paths below you in the hierarchy, and
> you had to get to your cwd _somehow_, at which point they could have
> attacked you then).

... eventually give this idea here a test?

Patrick