Hello,
A customer reported that AutoFS may leak file descriptors when some
maps are modified and the daemon reloaded. I'm able to reproduce the
problem on 5.0.7 by following these steps:
1. Configure a simple direct mount:
# cat /etc/auto.master
/- /etc/auto.direct
# cat /etc/auto.direct
/nfs server:/nfs
2. Start the automounter and do NOT trigger the mount
3. Replace /etc/auto.direct with:
# cat /etc/auto.direct
/nfs/1 server:/nfs
/nfs/2 server:/nfs
4. Reload:
# kill -HUP $(pidof automount)
>From now on, every reload will leak a file descriptor:
# ls -la /proc/$(pidof automount)/fd | grep /nfs
lr-x------ 1 root root 64 Aug 14 22:08 11 -> /nfs
lr-x------ 1 root root 64 Aug 14 22:08 12 -> /nfs
lr-x------ 1 root root 64 Aug 14 22:08 13 -> /nfs
lr-x------ 1 root root 64 Aug 14 22:08 14 -> /nfs
lr-x------ 1 root root 64 Aug 14 22:08 5 -> /nfs
I've investigated the problem and discovered that the leak happens in
do_umount_autofs_direct():
int do_umount_autofs_direct(struct autofs_point *ap, struct mnt_list
*mnts, struct mapent *me)
{
(...)
if (me->ioctlfd != -1) {
if (tree_is_mounted(mnts, me->key, MNTS_REAL)) {
error(ap->logopt,
"attempt to umount busy direct mount %s",
me->key);
return 1;
}
ioctlfd = me->ioctlfd;
} else // ioctlfd == -1
ops->open(ap->logopt, &ioctlfd, me->dev, me->key); <= we open it here
if (ioctlfd >= 0) {
unsigned int status = 1;
rv = ops->askumount(ap->logopt, ioctlfd, &status);
/// at this point, rv == 0 and status == 0
if (rv) {
char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
error(ap->logopt, "ioctl failed: %s", estr);
return 1;
} else if (!status) {
/// at this point, ap->state == ST_READMAP
if (ap->state != ST_SHUTDOWN_FORCE) {
error(ap->logopt,
"ask umount returned busy for %s",
me->key);
return 1; <= we return here, without closing the fd
} else {
me->ioctlfd = -1;
ops->catatonic(ap->logopt, ioctlfd);
ops->close(ap->logopt, ioctlfd);
goto force_umount;
}
(...)
I wrote a patch to workaround the leak because I still don't know how
to fix the root cause, which I believe is to remove all references to the
old map entry and unmount everything before creating the new entries.
There's also some open points in the workaround:
- should we call ops->catatonic() on the ioctlfd like it's done in the other
error paths?
- I've opted to close ioctlfd only if it was opened in this function. Is
this correct or should we close it regardless if it was opened before
the call?
Thanks,
Leonardo
Index: autofs-5.0.7/daemon/direct.c
===================================================================
--- autofs-5.0.7.orig/daemon/direct.c
+++ autofs-5.0.7/daemon/direct.c
@@ -87,6 +87,7 @@ int do_umount_autofs_direct(struct autof
struct ioctl_ops *ops = get_ioctl_ops();
char buf[MAX_ERR_BUF];
int ioctlfd, rv, left, retries;
+ int opened = 0;
left = umount_multi(ap, me->key, 0);
if (left) {
@@ -103,8 +104,10 @@ int do_umount_autofs_direct(struct autof
return 1;
}
ioctlfd = me->ioctlfd;
- } else
+ } else {
ops->open(ap->logopt, &ioctlfd, me->dev, me->key);
+ opened = 1;
+ }
if (ioctlfd >= 0) {
unsigned int status = 1;
@@ -119,6 +122,8 @@ int do_umount_autofs_direct(struct autof
error(ap->logopt,
"ask umount returned busy for %s",
me->key);
+ if (opened)
+ ops->close(ap->logopt, ioctlfd);
return 1;
} else {
me->ioctlfd = -1;
--
To unsubscribe from this list: send the line "unsubscribe autofs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
