All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] 5.0.0_beta1: use PATH in execXX()
@ 2007-09-26 10:00 Denys Vlasenko
  2007-09-26 15:11 ` Ian Kent
  2007-09-28  9:11 ` Ian Kent
  0 siblings, 2 replies; 4+ messages in thread
From: Denys Vlasenko @ 2007-09-26 10:00 UTC (permalink / raw)
  To: Ian Kent; +Cc: autofs

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

Hi Ian,

I want automount to use PATH when it tries to run e.g. "mount",
because I want it to work regardless where my mount binary is
(/bin or /usr/bin or...). I do have different system with different
paths to some binaries, and it is pain in the ass - I have
no way to explain to automount binary that it needs to look
in another directory.

I am trying to make automount using 'short' names it by passing
variables to configure:

MOUNT="mount" \
UMOUNT="umount" \
E2FSCK="fsck.ext2" \
E3FSCK="fsck.ext3" \
./configure ............

I think it's reasonably safe since automount is started by root
and root is expected to set PATH sanely.

And it still doesn't work, because you are using execv(), not execvp().

This patch replaces execv() with execvp().

It should change nothing for 'normally' configured automount since
it passes fully qualified names ("/bin/mount") to exec and PATH
is not used in this case.

Please consider applying.
--
vda

[-- Attachment #2: autofs-5.0.0_beta1-PATH.patch --]
[-- Type: text/x-diff, Size: 1642 bytes --]

diff -d -urpN autofs-5.0.0_beta1-stderr/daemon/spawn.c autofs-5.0.0_beta1-PATH/daemon/spawn.c
--- autofs-5.0.0_beta1-stderr/daemon/spawn.c	2006-05-02 08:37:35.000000000 +0100
+++ autofs-5.0.0_beta1-PATH/daemon/spawn.c	2007-09-26 10:35:59.000000000 +0100
@@ -291,8 +291,12 @@ static int do_spawn(logger *log, int use
 		dup2(pipefd[1], STDERR_FILENO);
 		close(pipefd[1]);
 
-		execv(prog, (char *const *) argv);
-		_exit(255);	/* execv() failed */
+		/* Why execv_p_? Normally argv[0] has fully qualified path
+		 * and it doesn't matter, but it can be overridden
+		 * at build time to e.g. simple "mount".
+		 * Clearly, user wants to use PATH in this case. */
+		execvp(prog, (char *const *) argv);
+		_exit(255);	/* exec() failed */
 	} else {
 		close(pipefd[1]);
 
diff -d -urpN autofs-5.0.0_beta1-stderr/modules/lookup_program.c autofs-5.0.0_beta1-PATH/modules/lookup_program.c
--- autofs-5.0.0_beta1-stderr/modules/lookup_program.c	2006-05-02 08:37:35.000000000 +0100
+++ autofs-5.0.0_beta1-PATH/modules/lookup_program.c	2007-09-26 10:36:24.000000000 +0100
@@ -171,8 +171,12 @@ int lookup_mount(struct autofs_point *ap
 		dup2(epipefd[1], STDERR_FILENO);
 		close(pipefd[1]);
 		close(epipefd[1]);
-		execl(ctxt->mapname, ctxt->mapname, name, NULL);
-		_exit(255);	/* execl() failed */
+		/* Why execl_p_? Normally argv[0] has fully qualified path
+		 * and it doesn't matter, but it can be overridden
+		 * at build time to e.g. simple "mount".
+		 * Clearly, user wants to use PATH in this case. */
+		execlp(ctxt->mapname, ctxt->mapname, name, NULL);
+		_exit(255);	/* exec() failed */
 	}
 	close(pipefd[1]);
 	close(epipefd[1]);

[-- Attachment #3: Type: text/plain, Size: 140 bytes --]

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] 5.0.0_beta1: use PATH in execXX()
  2007-09-26 10:00 [PATCH] 5.0.0_beta1: use PATH in execXX() Denys Vlasenko
@ 2007-09-26 15:11 ` Ian Kent
  2007-09-28  9:11 ` Ian Kent
  1 sibling, 0 replies; 4+ messages in thread
From: Ian Kent @ 2007-09-26 15:11 UTC (permalink / raw)
  To: Denys Vlasenko; +Cc: autofs

On Wed, 2007-09-26 at 11:00 +0100, Denys Vlasenko wrote:
> Hi Ian,
> 
> I want automount to use PATH when it tries to run e.g. "mount",
> because I want it to work regardless where my mount binary is
> (/bin or /usr/bin or...). I do have different system with different
> paths to some binaries, and it is pain in the ass - I have
> no way to explain to automount binary that it needs to look
> in another directory.

Yes, people have asked for this fairly often.
I've resisted so far but perhaps I'm just wrong to do so.
I'll have a look at this and think about it for a little while.

> 
> I am trying to make automount using 'short' names it by passing
> variables to configure:
> 
> MOUNT="mount" \
> UMOUNT="umount" \
> E2FSCK="fsck.ext2" \
> E3FSCK="fsck.ext3" \
> ./configure ............
> 
> I think it's reasonably safe since automount is started by root
> and root is expected to set PATH sanely.
> 
> And it still doesn't work, because you are using execv(), not execvp().
> 
> This patch replaces execv() with execvp().
> 
> It should change nothing for 'normally' configured automount since
> it passes fully qualified names ("/bin/mount") to exec and PATH
> is not used in this case.
> 
> Please consider applying.
> --
> vda

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] 5.0.0_beta1: use PATH in execXX()
  2007-09-26 10:00 [PATCH] 5.0.0_beta1: use PATH in execXX() Denys Vlasenko
  2007-09-26 15:11 ` Ian Kent
@ 2007-09-28  9:11 ` Ian Kent
  2007-09-28 16:53   ` Jim Carter
  1 sibling, 1 reply; 4+ messages in thread
From: Ian Kent @ 2007-09-28  9:11 UTC (permalink / raw)
  To: Denys Vlasenko; +Cc: autofs

On Wed, 2007-09-26 at 11:00 +0100, Denys Vlasenko wrote:
> I think it's reasonably safe since automount is started by root
> and root is expected to set PATH sanely.
> 
> And it still doesn't work, because you are using execv(), not execvp().
> 
> This patch replaces execv() with execvp().
> 
> It should change nothing for 'normally' configured automount since
> it passes fully qualified names ("/bin/mount") to exec and PATH
> is not used in this case.

I still can't agree that using $PATH to locate executable maps is OK.
If a system uses executable maps then I think the location of them
should be specified in full.

Ian

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] 5.0.0_beta1: use PATH in execXX()
  2007-09-28  9:11 ` Ian Kent
@ 2007-09-28 16:53   ` Jim Carter
  0 siblings, 0 replies; 4+ messages in thread
From: Jim Carter @ 2007-09-28 16:53 UTC (permalink / raw)
  To: Ian Kent; +Cc: autofs

On Fri, 28 Sep 2007, Ian Kent wrote:

> On Wed, 2007-09-26 at 11:00 +0100, Denys Vlasenko wrote:
> > I think it's reasonably safe since automount is started by root
> > and root is expected to set PATH sanely.
 --snip--
> I still can't agree that using $PATH to locate executable maps is OK.
> If a system uses executable maps then I think the location of them
> should be specified in full.

Well, in the development of autofs we started with file maps, and when 
executable maps were implemented it was natural to seem them as an 
evolution of file maps, i.e. properly found in /etc via a full path name.  
(/etc hasn't had generic executable content, like /etc/fsck, since 4.2BSD I 
believe).  However, the requester has given a plausible scenario, the same 
master map used on multiple distros with different preferred homes for 
locally hacked executable files.  Here at UCLA-Mathnet we had (in the past, 
thankfully) exactly this situation: Solaris, Irix, Ultrix and Linux at the 
same time and the same master map on all of them, although we stuck with 
the explicit path names in /etc.  (I think we may have had tweaks per OS, 
actually, which occasionally got overwritten when some bozo installed the 
master map "everwhere".)

My feeling is that programs should allow the sysop maximum flexibility for 
possibly unique situations, absent a real operational need for the more 
restrictive solution.  In other words, UNIX is supposed to let you shoot 
yourself in the foot.  So I support using execvp to exec executable maps, 
even though I definitely won't be taking advantage of the feature.

James F. Carter          Voice 310 825 2897    FAX 310 206 6673
UCLA-Mathnet;  6115 MSA; 405 Hilgard Ave.; Los Angeles, CA, USA  90095-1555
Email: jimc@math.ucla.edu    http://www.math.ucla.edu/~jimc (q.v. for PGP key)

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2007-09-28 16:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-26 10:00 [PATCH] 5.0.0_beta1: use PATH in execXX() Denys Vlasenko
2007-09-26 15:11 ` Ian Kent
2007-09-28  9:11 ` Ian Kent
2007-09-28 16:53   ` Jim Carter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.