Milos Nikic, le dim. 01 nov. 2026 22:28:23 -0800, a ecrit: > The full shape is: > > pthread_mutex_lock (&diskfs_root_node->lock); /* line 90 */ > ... > err = diskfs_lookup (diskfs_root_node, _diskfs_chroot_directory, > LOOKUP, &np, NULL, rootpi); > pthread_mutex_unlock (&diskfs_root_node->lock); /* line 102 */ > ... > old = diskfs_root_node; > diskfs_root_node = np; > if (np != old) > pthread_mutex_unlock (&np->lock); > diskfs_nrele (old); > > So old is diskfs_root_node, which is already unlocked at line 102 (of > the patched file). diskfs_nput (old) unlocked it a second time
Ah, so there was a double double-unlock. Applied, thanks! Samuel > (it > fails when the locks are made PTHREAD_MUTEX_ERRORCHECK); > only its reference is left to drop, hence diskfs_nrele. > > Alternatively, would it be simpler to just reject --virtual-root=. ? > Changing: > if (err == EAGAIN) > error (1, 0, "`--virtual-root=%s' specifies the real root directory", > _diskfs_chroot_directory); > > to something like: > > if (err == EAGAIN || (!err && np == diskfs_root_node)) > error (1, 0, "`--virtual-root=%s' specifies the real root directory", > _diskfs_chroot_directory); > > and then we wouldn't have this confusing case at all. > > Thanks, > Milos > > On Sun, Nov 1, 2026 at 10:49 AM Samuel Thibault <[1][email protected]> > wrote: > > Milos Nikic, le jeu. 29 oct. 2026 21:26:40 -0700, a ecrit: > > diskfs_startup_diskfs unlocks the root node after looking up the > > virtual root in it, then lets go of it with diskfs_nput, which unlocks > > it a second time. Release only its reference. > > > > With --virtual-root=., the lookup of "." gives back the root node > > itself, already unlocked with the root: do not unlock it again either. > > > > A default mutex does not check its owner, so the second unlock goes > > unnoticed unless another thread holds the lock by then, which loses it. > > With error-checking mutexes, an ext2fs started with --virtual-root=. > > dies at that unlock. > > --- > > libdiskfs/init-startup.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/libdiskfs/init-startup.c b/libdiskfs/init-startup.c > > index d51cc2787..736cdb502 100644 > > --- a/libdiskfs/init-startup.c > > +++ b/libdiskfs/init-startup.c > > @@ -120,8 +120,10 @@ diskfs_startup_diskfs (mach_port_t fs_bootstrap, > int > flags) > > if _diskfs_chroot_directory is non-null. */ > > old = diskfs_root_node; > > diskfs_root_node = np; > > - pthread_mutex_unlock (&np->lock); > > - diskfs_nput (old); > > + /* A virtual root that is the root itself was unlocked with it. > * > / > > + if (np != old) > > + pthread_mutex_unlock (&np->lock); > > + diskfs_nrele (old); > > Mmm, but then we don't unlock old? > > Samuel > > > References: > > [1] mailto:[email protected] -- Samuel Hi ! I'm a .signature virus ! Copy me into your ~/.signature, please !
