On Fri, Jan 31, 2020 at 07:16:14PM +0800, Ian Kent <ik...@redhat.com> wrote:
> 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.

Ah, I tried to see if there was a custom symlink patch in debians version,
but it seems to e integrated intot he source tree. I feel a bit guilty as I
think I am the likely reason why debian kept some symlink changes.

> >    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.

That's because the previous autofs maintainer, in true systemd fashioon,
decided that nobody should use symlinks as bind mounts are the future(tm).

This seesm to be no longer the case, and I greatly appreciate the apparent
change in policy to make symlinks viable - autofs would be completely
useless to me without symlink support.

> I definitely don't see the the target get umounted in the Fedora
> package or the current development source and I've verified the

I downlaoded autofs-5.1.5-10.fc30.src.rpm and indeed, this block is only in
the debian version (didn't check any .patch files in the rpm though):

        /* If path is a mount it can't be a symlink */
        if (is_mounted(_PATH_MOUNTED, path, MNTS_ALL))
                goto real_mount;

And this is exactly the code that causes this problem. is_mounted has
different semantics depending on whether /dev/autofs exists.

> 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.

Right. Even if this problem is fixed though, maybe some thought should
be given to the fact that is_mounted is not useful for symlinks in the
current version as the result is essentially random (practically, lstat
vs. stat).

MY hunch is that the kernel shouldn't follow symlinks when testing for
mountpoints, but that probably can't be changed anymore.

> > It happens with the debian package I am using, yes.
> 
> That's 5.1.2-4 right?

Yup!

-- 
                The choice of a       Deliantra, the free code+content MORPG
      -----==-     _GNU_              http://www.deliantra.net
      ----==-- _       generation
      ---==---(_)__  __ ____  __      Marc Lehmann
      --==---/ / _ \/ // /\ \/ /      schm...@schmorp.de
      -=====/_/_//_/\_,_/ /_/\_\

Reply via email to