Re: automount subprocesses accumulate

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 2015-09-13 at 13:20 +0200, Cyril B. wrote:
> Ian Kent wrote:
> > On Fri, 2015-09-11 at 17:14 +0200, Cyril B. wrote:
> >> >  Hello Ian,
> >> >
> >> >  Thanks for the quick response. I was able to reliably reproduce the
> >> >  deadlock on a test server with a test script that triggers many
> >> >  simultaneous mounts. On this setup /home is replaced with /mnt.
> >
> > Yes, it's a puzzle.
> >
> > The default.c module looks like there's no possibility of a deadlock.
> > It's fairly simple and there is no place where the mutex isn't released
> > before return and there are no other calls are made that could take
> > other locks that could introduce a deadlock.
> >
> > It occurred to me that the call to force_standard_program_map_env() is
> > out probably out of order.
> >
> > It's made after the fork() that's used to exec the program map code.
> >
> > I think the child is seeing the state of the mutex at the time of the
> > fork and if the mutex was locked at that time the child process will
> > never see it unlocked since the forked process copy will not see that
> > change.
> >
> > That's just a guess, so let me put together a patch for you to try.
> > Are you in a position to be able to apply and build autofs to test?
> 
> Sure, I can test as much as I want. I guess I could probably even setup 
> a test VM that reproduces the issue and give you root credentials if it 
> helps.
> 

OK, see if you can apply this patch.
The CHANGELOG hunk won't apply, of course, so just ignore the reject or
delete the hunk from the patch before applying it.

Umm ... now I see I need to fix a couple of spelling mistakes as
well, ;)

autofs-5.1.1 - fix out of order call in program map lookup

From: Ian Kent <raven@xxxxxxxxxx>

Commit 91e42e58b4 fixed a problem with program map environment variable
nameing and commit 743deb0e4e added a configuration option to force use
of the old environment names for those who need it and are sure it is
safe to continue to use them.

But the call to get the configuration entry was placed after a fork()
so the state of the mutex used when querying the configuration is
undefined and can lead to a dedlock.

Signed-off-by: Ian Kent <raven@xxxxxxxxxx>
---
 CHANGELOG                |    1 +
 modules/lookup_program.c |   20 ++++++++++----------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 4430c39..357d201 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 ??/??/2015 autofs-5.1.2
 =======================
 - fix left mount count return from umount_multi_triggers().
+- fix out of order call in program map lookup.
 
 21/04/2015 autofs-5.1.1
 =======================
diff --git a/modules/lookup_program.c b/modules/lookup_program.c
index a3a7e98..67d4c47 100644
--- a/modules/lookup_program.c
+++ b/modules/lookup_program.c
@@ -139,6 +139,16 @@ static char *lookup_one(struct autofs_point *ap,
 	}
 
 	/*
+	 * By default use a prefix with standard environment
+	 * variables to prevent system subversion by interpreted
+	 * languages.
+	 */
+	if (defaults_force_std_prog_map_env())
+		prefix = NULL;
+	else
+		prefix = "AUTOFS_";
+
+	/*
 	 * We don't use popen because we don't want to run /bin/sh plus we
 	 * want to send stderr to the syslog, and we don't use spawnl()
 	 * because we need the pipe hooks
@@ -177,16 +187,6 @@ static char *lookup_one(struct autofs_point *ap,
 			     ap->path, ctxt->mapname);
 
 		/*
-		 * By default use a prefix with standard environment
-		 * variables to prevent system subversion by interpreted
-		 * languages.
-		 */
-		if (defaults_force_std_prog_map_env())
-			prefix = NULL;
-		else
-			prefix = "AUTOFS_";
-
-		/*
 		 * MAPFMT_DEFAULT must be "sun" for ->parse_init() to have setup
 		 * the macro table.
 		 */


--
To unsubscribe from this list: send the line "unsubscribe autofs" in



[Index of Archives]     [Linux Filesystem Development]     [Linux Ext4]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux