On Fri, 2020-01-31 at 19:16 +0800, Ian Kent wrote:
> On Fri, 2020-01-31 at 07:55 +0100, Marc Lehmann wrote:
> > > > Sounds like I have enough information to duplicate the problem
> > > > so
> > > > I'll have a look see.
> > >
> > > Oh, as usual, what's the autofs source version of your package
> > > please,
> > > since I'll be using the current development source to work on it.
> >
> > It's the one mentioned in my original report (that you probably
> > didn't
> > get): 5.1.2-4
>
> I grabbed the Debian source package and unpacked it on an old Ubuntu
> VM I have.
>
> It looks like this package has the change that adds handling of
> symlinks to umount_multi() so this should not be calling umount()
> at all.
Ahh, Haa, I see the problem in the Debian package source.
As you say, there is a call to is_mounted() (which calls that ioctl)
at the top of umount_multi():
if (is_mounted(_PATH_MOUNTED, path, MNTS_ALL))
goto real_mount;
and that branches to the real mount umount code, this is now gone
in the current source.
I'll have a look at the change history and see if I can locate
the patch (or patches) that removed it.
>
> > On Fri, Jan 31, 2020 at 02:07:54PM +0800, Ian Kent <
> > ik...@redhat.com>
> > wrote:
> > > A quick test with the current Fedora autofs package, autofs-
> > > 5.1.5-
> > > 10.fc30.x86_64, I see that at expire the symlink is removed but
> > > the
> > > mount point directory used for browsing isn't recreated.
> > >
> > > I'll need to fix that.
> >
> > That's the minor aspect (But of course appreciqated :).
> >
> > > IIUC, the other aspect of this problem is that the target of the
> > > symlink, the NFSv4 mount, is also umounted at expire. That
> > > doesn't
> > > happen with the above Fedora package.
> >
> > The target (in my example) is not an NFS mount at all, it's just
> > normal
> > mountpoint, the target of the symlink (which in general might not
> > be
> > a but
> > mountpoint, in my case, is one).
> >
> > The problem is that autofs calls umount on the symlink (in my
> > example,
> > /fs/bp), but umount follows symlinks, so the target filesystem is
> > unnmounted,
> > if it is a mountpoint.
>
> That shouldn't be happening according to what I saw of the Debian
> package source, I guess I'll need to install Debian and try and
> duplicate the problem.
>
> > I tried with debug logging, and it looks like this on kill -USR1:
> >
> > expiring path /fs/bp
> > umount_multi: path /fs/bp incl 1
> > umount_multi: removing symlink /fs/bp
> > expired /fs/bp
> >
> > When stracing, it literally execs umount /fs/bp:
> >
> > 5463 execve("/bin/umount", ["/bin/umount", "/fs/bp"],
> > 0x5555555b23a0 /* 20 vars */) = 0
>
> Yeah, it may have done that before the symlink handling change
> I mentioned. For a long time there wasn't sufficient attention
> given to symlinks in favour of bind mounting.
>
> I definitely don't see the the target get umounted in the Fedora
> package or the current development source and I've verified the
> path the execution follows and that explicitly checks for a
> symlink, handles that case and returns, there's no exec of
> umount().
>
> > As explained earlier this only happens with misc-device support
> > because
> > autofs thinks /fs/bp is a mountpoint as the kernel seems to follow
> > symlinks as well - without /dev/autofs it uses another method which
> > correctly finds that /fs/bp is not a mountpoint, so umount is never
> > called.
>
> I could stop the path walk following symlinks but I'd rather not
> since the current code, including that of the Debian package (I'll
> look a bit closer at the Debian package source), has that early
> symlink handing I spoke about.
>
> > > This could get a bit complicated since there have been kernel
> > > changes as well as user space changes to the symlink handling
> > > with later version of autofs, we will need to work that out as
> > > we go.
> >
> > I don't think it's a new bug (or change) in the kernel, as I
> > remember
> > having had exactly this problem years ago (so long ago that I
> > forgot
> > about
> > it when confronted with the workaround, but it came back to me
> > quickly).
> >
> > But for the record, I am using linux 5.4.15.
> >
> > The reason I didn't report it at the time was that I binary-patched
> > the
> > debian package (with a regex-replace on mount_bind.so), and
> > obviously
> > all
> > bets are off with that method, and I didn't have time to reproduce
> > it
> > with
> > the pristine debian package, which I did now, however.
> >
> > > Can you confirm this second part of the problem occurs with the
> > > Debian package your using please, then we can hunt for patches
> > > from later autofs releases.
> >
> > It happens with the debian package I am using, yes.
>
> That's 5.1.2-4 right?
>
> So maybe I've not understood the problem properly.
> I'll need to install Debian, then we can talk on the same terms.
>
> I'll get back after I've done this (and looked closer at the source
> used by the Debian package ...
>
> Ian
