Bug#950287: autofs: automount expriing unmounts target filesystem
On Sat, 2020-02-01 at 22:39 +0100, Marc Lehmann wrote: > On Sat, Feb 01, 2020 at 07:41:03AM +0800, Ian Kent > wrote: > > > decided that nobody should use symlinks as bind mounts are the > > > future(tm). > > > From an upstream POV it was more neglect (on my part) of the > > symlink code in favour of bind mounts. > > Not sure why you write this, but it's clearly not true: > >Date: Tue, 29 May 2001 14:00:42 -0700 >[...] >I don't want to perpetuate the symlink thing because it is a > terrible >hack in the kernel part of the code, and thus will be removed in > autofs >v5. > >-hpa > > (I hope he forgives me to quote a private mail, but the fact is > documented > in debian bug #128171). > > That was probably a bit before your time, but the last time I > reported > problems with symlinks to upstream I was told (in a long thread) that > it > is a hack and will not be supported. Yes, it was before my time and things have changed a lot. TBH I'm not sure what hpa was referring to there. He may have been talking about the abuse of symlink following in the VFS to effect automounting, but that's long since been resolved by adding automount support to the VFS itself, not only for use by autofs but other file systems like NFS, CIFS, AFS, etc.. > > And the result was that the debian maintainer locally applied a patch > (in > 2006) to make symlinks configurable, for which I was super-thankful. > > (Note that there are two sides to this: symlinks for local nfs > mounts, and > symlinks for bind mounts, which are not the same, and which confused > me for > quite a while). > > > But the autofs amd format map support needs them to work properly > > so quite a bit of effort went into making them work as best as I > > could. > > Yes, I am very fond of this change in policy, thanks a lot - the > effect is > that I can consider autofs again for new deployments, rather than > having > had to give up on it :) I also am very fond of amd map support, > although > documentation, of course, is sorely lacking. Yeah, but the amd support is very much for people that can't convert their maps to autofs format to get the same function. You can always look to: https://www.am-utils.org/docs/am-utils/am-utils.pdf while it's still around. Then it's just a matter of working out what's not supported in autofs and that's not all that much. The main problem with using autofs with amd maps is the way autofs works internally, it's different to amd so you end up with more actual mounts which isn't ideal from an amd POV. > > In any case, since we all seems to agree on the bug part, I think > this > part of the disucssion (who stated what policy when) should stay off > the > debian bts :) Agreed, the autofs mailing list is the better place for these discussions, ;) Ian
Bug#950287: autofs: automount expriing unmounts target filesystem
On Sat, Feb 01, 2020 at 07:41:03AM +0800, Ian Kent wrote: > > decided that nobody should use symlinks as bind mounts are the > > future(tm). > > >From an upstream POV it was more neglect (on my part) of the > symlink code in favour of bind mounts. Not sure why you write this, but it's clearly not true: Date: Tue, 29 May 2001 14:00:42 -0700 [...] I don't want to perpetuate the symlink thing because it is a terrible hack in the kernel part of the code, and thus will be removed in autofs v5. -hpa (I hope he forgives me to quote a private mail, but the fact is documented in debian bug #128171). That was probably a bit before your time, but the last time I reported problems with symlinks to upstream I was told (in a long thread) that it is a hack and will not be supported. And the result was that the debian maintainer locally applied a patch (in 2006) to make symlinks configurable, for which I was super-thankful. (Note that there are two sides to this: symlinks for local nfs mounts, and symlinks for bind mounts, which are not the same, and which confused me for quite a while). > But the autofs amd format map support needs them to work properly > so quite a bit of effort went into making them work as best as I > could. Yes, I am very fond of this change in policy, thanks a lot - the effect is that I can consider autofs again for new deployments, rather than having had to give up on it :) I also am very fond of amd map support, although documentation, of course, is sorely lacking. In any case, since we all seems to agree on the bug part, I think this part of the disucssion (who stated what policy when) should stay off the debian bts :) -- The choice of a Deliantra, the free code+content MORPG -==- _GNU_ http://www.deliantra.net ==-- _ generation ---==---(_)__ __ __ Marc Lehmann --==---/ / _ \/ // /\ \/ / schm...@schmorp.de -=/_/_//_/\_,_/ /_/\_\
Bug#950287: autofs: automount expriing unmounts target filesystem
On Fri, 2020-01-31 at 18:15 +0100, Marc Lehmann wrote:
> On Fri, Jan 31, 2020 at 07:16:14PM +0800, Ian Kent
> 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"],
> > > 0x555b23a0 /* 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).
>From an upstream POV it was more neglect (on my part) of the
symlink code in favour of bind mounts.
But the autofs amd format map support needs them to work properly
so quite a bit of effort went into making them work as best as I
could.
>
> 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.
Right, apologies for that.
>
> > 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.
Right, if /dev/autofs does not exist then is_mounted() will consult
the text based mount tables. That has terrible performance when the
mount table is large so not using the miscellaneous device isn't the
best idea in general.
>
> > 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.
I am thinking about it.
The is_mounted() function is used in numerous places and I'm pretty
sure there are cases where it needs to follow symlinks so that's
tricky.
Ian
Bug#950287: autofs: automount expriing unmounts target filesystem
Hi all, On Fr 31 Jan 2020 18:24:42 CET, Marc Lehmann wrote: On Fri, Jan 31, 2020 at 02:24:11PM +, Mike Gabriel wrote: Marc, as I get it, it would be welcome to get the discussed issue fixed in Debian stable (aka buster). Right? It would be convenient yes, but since I had to roll back the timeout change (in my configs) anyway, I can live with the current version for a few more years, and it probably only affects a small subset of hosts anyway, as most only have a symlink to / which won't be unmounted aciidentally... The fact that upstream apparently changed policy on symlinks (to "supported", probably years ago, but I haven't known) is the most welcome news for me! Maybe some day I finally have a replacement for amd that can actually be restarted cleanly without rebooting the machine :) thanks for the feedback. Ok then, I won't push that into buster-proposed-updates, then. It will reach you with bullseye... Mike -- mike gabriel aka sunweaver (Debian Developer) mobile: +49 (1520) 1976 148 landline: +49 (4351) 486 14 27 GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22 0782 9AF4 6B30 2577 1B31 mail: sunwea...@debian.org, http://sunweavers.net pgpyEjf7Qoskv.pgp Description: Digitale PGP-Signatur
Bug#950287: autofs: automount expriing unmounts target filesystem
As added info, I installed 5.1.6 form tetsing on some of the affected machines, and the problem is indeed gone, local filesystems don't randomly get unmounted on autofs restarts or expires. Thanks! -- The choice of a Deliantra, the free code+content MORPG -==- _GNU_ http://www.deliantra.net ==-- _ generation ---==---(_)__ __ __ Marc Lehmann --==---/ / _ \/ // /\ \/ / schm...@schmorp.de -=/_/_//_/\_,_/ /_/\_\
Bug#950287: autofs: automount expriing unmounts target filesystem
On Fri, Jan 31, 2020 at 02:24:11PM +, Mike Gabriel wrote: > Marc, as I get it, it would be welcome to get the discussed issue fixed in > Debian stable (aka buster). Right? It would be convenient yes, but since I had to roll back the timeout change (in my configs) anyway, I can live with the current version for a few more years, and it probably only affects a small subset of hosts anyway, as most only have a symlink to / which won't be unmounted aciidentally... The fact that upstream apparently changed policy on symlinks (to "supported", probably years ago, but I haven't known) is the most welcome news for me! Maybe some day I finally have a replacement for amd that can actually be restarted cleanly without rebooting the machine :) -- The choice of a Deliantra, the free code+content MORPG -==- _GNU_ http://www.deliantra.net ==-- _ generation ---==---(_)__ __ __ Marc Lehmann --==---/ / _ \/ // /\ \/ / schm...@schmorp.de -=/_/_//_/\_,_/ /_/\_\
Bug#950287: autofs: automount expriing unmounts target filesystem
On Fri, Jan 31, 2020 at 07:16:14PM +0800, Ian Kent 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"],
> > 0x555b23a0 /* 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
-=/_/_//_/\_,_/ /_/\_\
Bug#950287: autofs: automount expriing unmounts target filesystem
Control: close -1 Control: fixed -1 5.1.5-1 Hi, On Fr 31 Jan 2020 12:47:15 CET, Ian Kent wrote: On Fri, 2020-01-31 at 19:35 +0800, Ian Kent wrote: On Fri, 2020-01-31 at 19:29 +0800, Ian Kent wrote: > 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. There it is, and it looks independent of other changes ... autofs-5.1.3 - fix symlink false negative in umount_multi() And you can find this at: https://mirrors.edge.kernel.org/pub/linux/daemons/autofs/v5/patches-5.1.4/autofs-5.1.3-fix-symlink-false-negative-in-umount_multi.patch Ian Thanks for clarifying, Ian. Much appreciated. I have marked the issue as resolved in Debian for autofs 5.1.5-1 (there were no 5.1.3 / 5.1.4 uploads in Debian due to orphaned package state). Marc, as I get it, it would be welcome to get the discussed issue fixed in Debian stable (aka buster). Right? Mike -- mike gabriel aka sunweaver (Debian Developer) mobile: +49 (1520) 1976 148 landline: +49 (4351) 486 14 27 GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22 0782 9AF4 6B30 2577 1B31 mail: sunwea...@debian.org, http://sunweavers.net pgppSgipYDhTx.pgp Description: Digitale PGP-Signatur
Bug#950287: autofs: automount expriing unmounts target filesystem
On Fri, 2020-01-31 at 19:35 +0800, Ian Kent wrote: > On Fri, 2020-01-31 at 19:29 +0800, Ian Kent wrote: > > 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. > > There it is, and it looks independent of other changes ... > > autofs-5.1.3 - fix symlink false negative in umount_multi() And you can find this at: https://mirrors.edge.kernel.org/pub/linux/daemons/autofs/v5/patches-5.1.4/autofs-5.1.3-fix-symlink-false-negative-in-umount_multi.patch Ian
Bug#950287: autofs: automount expriing unmounts target filesystem
On Fri, 2020-01-31 at 19:29 +0800, Ian Kent wrote:
> 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.
There it is, and it looks independent of other changes ...
autofs-5.1.3 - fix symlink false negative in umount_multi()
From: Ian Kent
Using is_mounted() on a symlink will give a false negative for MNTS_ALL
since the kernel will return mounted true for a dentry within an autofs
mount point.
Just remove the check and rely on lstat().
Signed-off-by: Ian Kent
---
CHANGELOG |1 +
daemon/automount.c |5 -
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/CHANGELOG b/CHANGELOG
index d6016984..df87df49 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -45,6 +45,7 @@ xx/xx/2017 autofs-5.1.4
- improve debug logging of lookup key.
- fix typo in amd_parse.c.
- add missing MODPREFIX to logging in amd parser.
+- fix symlink false negative in umount_multi().
24/05/2017 autofs-5.1.3
===
diff --git a/daemon/automount.c b/daemon/automount.c
index 5c1481c4..3ff4f778 100644
--- a/daemon/automount.c
+++ b/daemon/automount.c
@@ -633,10 +633,6 @@ int umount_multi(struct autofs_point *ap, const char
*path, int incl)
debug(ap->logopt, "path %s incl %d", path, incl);
- /* If path is a mount it can't be a symlink */
- if (is_mounted(_PATH_MOUNTED, path, MNTS_ALL))
- goto real_mount;
-
if (lstat(path, )) {
warn(ap->logopt,
"failed to stat directory or symlink %s", path);
@@ -681,7 +677,6 @@ int umount_multi(struct autofs_point *ap, const char *path,
int incl)
return 0;
}
-real_mount:
is_autofs_fs = 0;
if (master_find_submount(ap, path))
is_autofs_fs = 1;
Bug#950287: autofs: automount expriing unmounts target filesystem
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"],
> > 0x555b23a0 /* 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
Bug#950287: autofs: automount expriing unmounts target filesystem
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.
>
> On Fri, Jan 31, 2020 at 02:07:54PM +0800, Ian Kent
> 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"],
> 0x555b23a0 /* 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
Bug#950287: autofs: automount expriing unmounts target filesystem
> > 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
On Fri, Jan 31, 2020 at 02:07:54PM +0800, Ian Kent 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.
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"], 0x555b23a0 /* 20
vars */) = 0
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.
> 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.
--
The choice of a Deliantra, the free code+content MORPG
-==- _GNU_ http://www.deliantra.net
==-- _ generation
---==---(_)__ __ __ Marc Lehmann
--==---/ / _ \/ // /\ \/ / schm...@schmorp.de
-=/_/_//_/\_,_/ /_/\_\
Bug#950287: autofs: automount expriing unmounts target filesystem
On Fri, 2020-01-31 at 13:34 +0800, Ian Kent wrote: > On Fri, 2020-01-31 at 01:09 +0100, Marc Lehmann wrote: > > On Thu, Jan 30, 2020 at 11:50:32PM +, Mike Gabriel < > > sunwea...@debian.org> wrote: > > > This seems like something to be discussed upstream. I Cc:ed Ian > > > Kent, maybe > > > he has some 2? to add. > > > > Cool! > > > > In the meantime, I digged a bit more, and it seems that it is > > because > > with > > misc-device support, autofs uses the AUTOFS_DEV_IOCTL_ISMOUNTPOINT > > ioctl > > to check whether something is a mountpoint, which returns 1, which > > causes > > the code in umount_multi to decude it cannot be symlink and > > unmounting it. > > > > Indeed, rm /dev/autofs before starting autofs makes it partially > > work > > (the > > target dir is not unmounted, but the entry is still gone without > > ghosting). > > > > Upstream probably already knows this, of course. > > Upstream doesn't know about this, ;) > > The behaviour is not correct and needs to be fixed. > > Sounds like I have enough information to duplicate the problem so > I'll have a look see. 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. 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. 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. 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. Ian
Bug#950287: autofs: automount expriing unmounts target filesystem
On Fri, 2020-01-31 at 13:34 +0800, Ian Kent wrote: > On Fri, 2020-01-31 at 01:09 +0100, Marc Lehmann wrote: > > On Thu, Jan 30, 2020 at 11:50:32PM +, Mike Gabriel < > > sunwea...@debian.org> wrote: > > > This seems like something to be discussed upstream. I Cc:ed Ian > > > Kent, maybe > > > he has some 2? to add. > > > > Cool! > > > > In the meantime, I digged a bit more, and it seems that it is > > because > > with > > misc-device support, autofs uses the AUTOFS_DEV_IOCTL_ISMOUNTPOINT > > ioctl > > to check whether something is a mountpoint, which returns 1, which > > causes > > the code in umount_multi to decude it cannot be symlink and > > unmounting it. > > > > Indeed, rm /dev/autofs before starting autofs makes it partially > > work > > (the > > target dir is not unmounted, but the entry is still gone without > > ghosting). > > > > Upstream probably already knows this, of course. > > Upstream doesn't know about this, ;) > > The behaviour is not correct and needs to be fixed. > > 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. > > Thanks for the report > Ian
Bug#950287: autofs: automount expriing unmounts target filesystem
On Fri, 2020-01-31 at 01:09 +0100, Marc Lehmann wrote: > On Thu, Jan 30, 2020 at 11:50:32PM +, Mike Gabriel < > sunwea...@debian.org> wrote: > > This seems like something to be discussed upstream. I Cc:ed Ian > > Kent, maybe > > he has some 2? to add. > > Cool! > > In the meantime, I digged a bit more, and it seems that it is because > with > misc-device support, autofs uses the AUTOFS_DEV_IOCTL_ISMOUNTPOINT > ioctl > to check whether something is a mountpoint, which returns 1, which > causes > the code in umount_multi to decude it cannot be symlink and > unmounting it. > > Indeed, rm /dev/autofs before starting autofs makes it partially work > (the > target dir is not unmounted, but the entry is still gone without > ghosting). > > Upstream probably already knows this, of course. Upstream doesn't know about this, ;) The behaviour is not correct and needs to be fixed. Sounds like I have enough information to duplicate the problem so I'll have a look see. Thanks for the report Ian
Bug#950287: autofs: automount expriing unmounts target filesystem
On Thu, Jan 30, 2020 at 11:50:32PM +, Mike Gabriel wrote: > This seems like something to be discussed upstream. I Cc:ed Ian Kent, maybe > he has some 2? to add. Cool! In the meantime, I digged a bit more, and it seems that it is because with misc-device support, autofs uses the AUTOFS_DEV_IOCTL_ISMOUNTPOINT ioctl to check whether something is a mountpoint, which returns 1, which causes the code in umount_multi to decude it cannot be symlink and unmounting it. Indeed, rm /dev/autofs before starting autofs makes it partially work (the target dir is not unmounted, but the entry is still gone without ghosting). Upstream probably already knows this, of course. -- The choice of a Deliantra, the free code+content MORPG -==- _GNU_ http://www.deliantra.net ==-- _ generation ---==---(_)__ __ __ Marc Lehmann --==---/ / _ \/ // /\ \/ / schm...@schmorp.de -=/_/_//_/\_,_/ /_/\_\
Bug#950287: autofs: automount expriing unmounts target filesystem
Hi Marc, hi Ian, @Ian: I am the new autofs package maintainer in Debian. On Fr 31 Jan 2020 00:38:10 CET, Marc Lehmann wrote: Dear Maintainer, for many years, I started automount with -t 8640, and by now had forgotten what this was for (or whether I reported the problem). Today, I removed it and got reminded what it was supposed to work around. Effectively, when automount expires mountpoints, specifically symlink bind mounts, it will unmount the target directory. Example, with browse mode being on: auto.master: /fs /etc/auto.fs symlink -vers=4,intr,rsize=1048576,wsize=1048576,tcp,port=2049,fsc auto.fs: bp -fstype=bind:/bp With this setup, doing ls /fs show a bp directory. ls /fs/bp will replace it by a symlink to /bp, then killall -USR1 automount will remove /fs/bp (which I think it shouldn't when in browse mode, bug #1) and ALSO unmount /bp (bug #2, the real problem). I guess automount simply calls umount on the symlink and then unlinks it. killall -HUP automount will bring back the ghosted /fs/bp entry. alternatively, ls /fs/bp will bring bakc the symolink, butg of course the target directory stays unmounted. I think I never reported it because this behaviour was with my own patched copy, but it is reproducible with the pristine version. This seems like something to be discussed upstream. I Cc:ed Ian Kent, maybe he has some 2ยข to add. Thanks+Greets, Mike -- mike gabriel aka sunweaver (Debian Developer) mobile: +49 (1520) 1976 148 landline: +49 (4351) 486 14 27 GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22 0782 9AF4 6B30 2577 1B31 mail: sunwea...@debian.org, http://sunweavers.net pgpT3eFeAT7e_.pgp Description: Digitale PGP-Signatur
Bug#950287: autofs: automount expriing unmounts target filesystem
Package: autofs Version: 5.1.2-4 Severity: normal Dear Maintainer, for many years, I started automount with -t 8640, and by now had forgotten what this was for (or whether I reported the problem). Today, I removed it and got reminded what it was supposed to work around. Effectively, when automount expires mountpoints, specifically symlink bind mounts, it will unmount the target directory. Example, with browse mode being on: auto.master: /fs /etc/auto.fs symlink -vers=4,intr,rsize=1048576,wsize=1048576,tcp,port=2049,fsc auto.fs: bp -fstype=bind:/bp With this setup, doing ls /fs show a bp directory. ls /fs/bp will replace it by a symlink to /bp, then killall -USR1 automount will remove /fs/bp (which I think it shouldn't when in browse mode, bug #1) and ALSO unmount /bp (bug #2, the real problem). I guess automount simply calls umount on the symlink and then unlinks it. killall -HUP automount will bring back the ghosted /fs/bp entry. alternatively, ls /fs/bp will bring bakc the symolink, butg of course the target directory stays unmounted. I think I never reported it because this behaviour was with my own patched copy, but it is reproducible with the pristine version. -- System Information: Debian Release: 10.2 APT prefers stable APT policy: (990, 'stable'), (500, 'unstable-debug'), (500, 'testing-debug'), (500, 'stable-updates'), (500, 'stable-debug'), (500, 'oldstable-updates'), (500, 'oldstable-debug'), (500, 'unstable'), (500, 'testing'), (500, 'oldstable'), (1, 'experimental-debug'), (1, 'experimental') Architecture: amd64 (x86_64) Foreign Architectures: i386, x32 Kernel: Linux 5.4.15-050415-generic (SMP w/8 CPU cores) Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE Locale: LANG=en_DK.UTF-8, LC_CTYPE=en_DK.UTF-8 (charmap=UTF-8), LANGUAGE=en_DK.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /usr/bin/dash Init: systemd (via /run/systemd/system) LSM: AppArmor: enabled Versions of packages autofs depends on: ii libc62.28-10 ii libxml2 2.9.4+dfsg1-7+b3 ii ucf 3.0038+nmu1 Versions of packages autofs recommends: ii e2fsprogs 1.45.5-2 ii kmod26-1 ii nfs-common 1:1.3.4-2.5 autofs suggests no packages. -- debconf information excluded
