kern/59627: busy-waiting in onionfs

[dholland%NetBSD.org@localhost]

>Number:         59627
>Category:       kern
>Synopsis:       busy-waiting in onionfs
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Sep 02 20:50:00 +0000 2025
>Originator:     David A. Holland
>Release:        NetBSD 11.99.1 (20250902)
>Organization:
>Environment:
 n/a
>Description:

union_lookup1 contains this gem:

        while (dvp != udvp && (dvp->v_type == VDIR) &&
               (mp = dvp->v_mountedhere)) {
                if (vfs_busy(mp))
                        continue;

Note that the only way vfs_busy() fails is if the mount point is
partway through being unmounted; however, in that case (assuming it's
actually possible at all these days, not a given) we'll sit here
spinning in a tight loop until dvp->v_mountedhere clears.

This is not desirable.

Also, properly speaking this needs a membar_datadep_consumer() between
the read of dvp->v_mountedhere and the check of mp->mnt_iflag in
vfs_busy, which is missing, although realistically that's probably
only the first of a range of possible races.

>How-To-Repeat:

Inspection.

>Fix:

It isn't obvious to me why this code is here at all. Mount points
returned by VOP_LOOKUP are resolved in the vfs layer. It might be that
onionfs is trying to let you have one onionfs on top of a whole mount
hierarchy (and I suppose some such functionality is needed if you want
to be able to use it to merge subtrees with mount points where the
mount points don't line up) but... the layerfs mechanism is not meant
to handle that and it seems virtually guaranteed to not work.

So it might be sufficient to just remove the whole code block.

Alternatively this should probably be switched to the fstrans-based
method used by the vfs-level code in vfs_lookup.c.