question about hardcoded binary paths (swapon / mkswap)

question about hardcoded binary paths (swapon / mkswap)

[Ruediger Meier]

Hi,

I wonder about some hardcoded binary paths.

Example swapon.c:

#define PATH_MKSWAP    "/sbin/mkswap"

There are a two problems.
1. It's wrong. We should use $sbindir from configure.
2. When called from our test-suite it will use a wrong (or
   non-existend, broken) binary. This happens in test swapon/fixpgsz.

The question is how to fix this.

I would prefer to use "mkwsap" from the same directory like swapon or to 
simply execvp "mkswap" from PATH. But don't know if we want this. If we 
really want to keep a hardcoded sbindir then we would need "#ifdef 
TEST_PROGRAM".

Any comments?

cu,
Rudi

Re: question about hardcoded binary paths (swapon / mkswap)

[Isaac Dunham]

On Wed, Apr 01, 2015 at 01:42:56PM +0200, Ruediger Meier wrote:
> Hi,
> 
> I wonder about some hardcoded binary paths.
> 
> Example swapon.c:
> 
> #define PATH_MKSWAP    "/sbin/mkswap"
> 
> There are a two problems.
> 1. It's wrong. We should use $sbindir from configure.
> 2. When called from our test-suite it will use a wrong (or
>    non-existend, broken) binary. This happens in test swapon/fixpgsz.
> 
> The question is how to fix this.
> 
> I would prefer to use "mkwsap" from the same directory like swapon or to 
> simply execvp "mkswap" from PATH. But don't know if we want this. If we 
> really want to keep a hardcoded sbindir then we would need "#ifdef 
> TEST_PROGRAM".
> 
> Any comments?

The approach that seems obvious to me (assuming you want to keep the
hardcoded path) is:
-add -DSBINDIR="$sbindir" to CFLAGS
-in the testsuite, run tests in a private mount namespace, where you can
bind-mount $sbindir.

However, I'm guessing this would have to be done with a union mount, 
and there are probably problems like what to do if sbindir is 
non-existent down to / (eg, sbindir=/util-linux/rootcommmands on a 
standard Linux) -- creating a union mount over / may be a Bad Thing.

Thanks,
Isaac Dunham

Re: question about hardcoded binary paths (swapon / mkswap)

[Ruediger Meier]

On Wednesday 01 April 2015, Isaac Dunham wrote:
> On Wed, Apr 01, 2015 at 01:42:56PM +0200, Ruediger Meier wrote:
> > Hi,
> >
> > I wonder about some hardcoded binary paths.
> >
> > Example swapon.c:
> >
> > #define PATH_MKSWAP    "/sbin/mkswap"
> >
> > There are a two problems.
> > 1. It's wrong. We should use $sbindir from configure.
> > 2. When called from our test-suite it will use a wrong (or
> >    non-existend, broken) binary. This happens in test
> > swapon/fixpgsz.
> >
> > The question is how to fix this.
> >
> > I would prefer to use "mkwsap" from the same directory like swapon
> > or to simply execvp "mkswap" from PATH. But don't know if we want
> > this. If we really want to keep a hardcoded sbindir then we would
> > need "#ifdef TEST_PROGRAM".
> >
> > Any comments?
>
> The approach that seems obvious to me (assuming you want to keep the
> hardcoded path) is:
> -add -DSBINDIR="$sbindir" to CFLAGS

Yes, this would be easy. But my preferred logic would be like this

If "swapon" was called from PATH then just take mkswap from PATH too.
If "/whatever/path/swapon" was called then look for mkswap in the same 
path.

Maybe both cases also with or without fallback $sbindir, /sbin or $PATH.

I guess we should agree how somthing like this should be handeled in 
general. "eject" is also using hardcoded "/bin/umount". 

> -in the testsuite, run tests in a private mount namespace, where you
> can bind-mount $sbindir.
>
> However, I'm guessing this would have to be done with a union mount,
> and there are probably problems like what to do if sbindir is
> non-existent down to / (eg, sbindir=/util-linux/rootcommmands on a
> standard Linux) -- creating a union mount over / may be a Bad Thing.

Ah interesting, somthing like this would be nice to run all the tests 
for a real installtion!

Instead of "union mount over /" you could do it over a bind mount of /.
Then "make install DESTDIR=/mnt/union_mount/" and 
chroot /mnt/union_mount/ to run the tests there.

But usually I run the tests on virtual machines anyway where a 
real "make install" is possible. Just our test-suite would need the 
feature to use the installed binarys only. Should be simple to add.

cu,
Rudi

Re: question about hardcoded binary paths (swapon / mkswap)

[Mike Frysinger]

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

On 01 Apr 2015 18:17, Ruediger Meier wrote:
> On Wednesday 01 April 2015, Isaac Dunham wrote:
> > On Wed, Apr 01, 2015 at 01:42:56PM +0200, Ruediger Meier wrote:
> > > I wonder about some hardcoded binary paths.
> > >
> > > Example swapon.c:
> > >
> > > #define PATH_MKSWAP    "/sbin/mkswap"
> > >
> > > There are a two problems.
> > > 1. It's wrong. We should use $sbindir from configure.
> > > 2. When called from our test-suite it will use a wrong (or
> > >    non-existend, broken) binary. This happens in test
> > > swapon/fixpgsz.
> > >
> > > The question is how to fix this.
> > >
> > > I would prefer to use "mkwsap" from the same directory like swapon
> > > or to simply execvp "mkswap" from PATH. But don't know if we want
> > > this. If we really want to keep a hardcoded sbindir then we would
> > > need "#ifdef TEST_PROGRAM".
> > >
> > > Any comments?
> >
> > The approach that seems obvious to me (assuming you want to keep the
> > hardcoded path) is:
> > -add -DSBINDIR="$sbindir" to CFLAGS
> 
> Yes, this would be easy. But my preferred logic would be like this
> 
> If "swapon" was called from PATH then just take mkswap from PATH too.
> If "/whatever/path/swapon" was called then look for mkswap in the same 
> path.
> 
> Maybe both cases also with or without fallback $sbindir, /sbin or $PATH.
> 
> I guess we should agree how somthing like this should be handeled in 
> general. "eject" is also using hardcoded "/bin/umount". 

seems like $PATH should always be used.  if you broke $PATH, well that's your 
fualt ... tools shouldn't generally be expected to work in the fact of hostile 
environments like this.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: question about hardcoded binary paths (swapon / mkswap)

[Ruediger Meier]

On Wednesday 01 April 2015, Mike Frysinger wrote:
> On 01 Apr 2015 18:17, Ruediger Meier wrote:
> > On Wednesday 01 April 2015, Isaac Dunham wrote:
> > > On Wed, Apr 01, 2015 at 01:42:56PM +0200, Ruediger Meier wrote:
> > > > I wonder about some hardcoded binary paths.
> > > >
> > > > Example swapon.c:
> > > >
> > > > #define PATH_MKSWAP    "/sbin/mkswap"
> > > >
> > > > There are a two problems.
> > > > 1. It's wrong. We should use $sbindir from configure.
> > > > 2. When called from our test-suite it will use a wrong (or
> > > >    non-existend, broken) binary. This happens in test
> > > > swapon/fixpgsz.
> > > >
> > > > The question is how to fix this.
> > > >
> > > > I would prefer to use "mkwsap" from the same directory like
> > > > swapon or to simply execvp "mkswap" from PATH. But don't know
> > > > if we want this. If we really want to keep a hardcoded sbindir
> > > > then we would need "#ifdef TEST_PROGRAM".
> > > >
> > > > Any comments?
> > >
> > > The approach that seems obvious to me (assuming you want to keep
> > > the hardcoded path) is:
> > > -add -DSBINDIR="$sbindir" to CFLAGS
> >
> > Yes, this would be easy. But my preferred logic would be like this
> >
> > If "swapon" was called from PATH then just take mkswap from PATH
> > too. If "/whatever/path/swapon" was called then look for mkswap in
> > the same path.
> >
> > Maybe both cases also with or without fallback $sbindir, /sbin or
> > $PATH.
> >
> > I guess we should agree how somthing like this should be handeled
> > in general. "eject" is also using hardcoded "/bin/umount".
>
> seems like $PATH should always be used.  if you broke $PATH, well
> that's your fualt ... tools shouldn't generally be expected to work
> in the fact of hostile environments like this.
> -mike

I'd also like to use PATH (at least as fallback).

The only special thing here is IMO that mkswap and swapon belong to the 
same project. Should we try to use the right one per default?

Example:
UL is installed below /usr/local
PATH="/usr/local/sbin:/sbin"

Explicitly invoking the (old) globally installed /sbin/swapon should use 
the old /sbin/mkswap too or the new /usr/local/sbin/mkswap from PATH? 

Another point:
If "/sbin" is not in PATH should "sudo /sbin/swapon" find /sbin/mkswap 
or not?

cu,
Rudi

Re: question about hardcoded binary paths (swapon / mkswap)

[Karel Zak]

On Wed, Apr 01, 2015 at 10:06:52PM +0100, Ruediger Meier wrote:
> > > If "swapon" was called from PATH then just take mkswap from PATH
> > > too. If "/whatever/path/swapon" was called then look for mkswap in
> > > the same path.

It seems like over-engineering. The primary goal are regular
installations, the stuff in the test/ should not be a reason to change
important things in the utils. 

> > > Maybe both cases also with or without fallback $sbindir, /sbin or
> > > $PATH.
> > >
> > > I guess we should agree how somthing like this should be handeled
> > > in general. "eject" is also using hardcoded "/bin/umount".
> >
> > seems like $PATH should always be used.  if you broke $PATH, well

Yes, agree.
 
Note that we already have and use FS_SEARCH_PATH in mkfs, fsck and
mount (libmount), see --enable-fs-paths-default and  --enable-fs-paths-extra.

Maybe we can use it use FS_SEARCH_PATH also for mkswap in swapon, or use it
as fallback.

> The only special thing here is IMO that mkswap and swapon belong to the 
> same project. Should we try to use the right one per default?
> 
> Example:
> UL is installed below /usr/local
> PATH="/usr/local/sbin:/sbin"
> 
> Explicitly invoking the (old) globally installed /sbin/swapon should use 
> the old /sbin/mkswap too or the new /usr/local/sbin/mkswap from PATH? 

I'd like to avoid complex and not obvious semantic. It's fine to
follow PATH or harcoded FS_SEARCH_PATH.

> Another point:
> If "/sbin" is not in PATH should "sudo /sbin/swapon" find /sbin/mkswap 
> or not?

Is it really our problem? I don't think we have to provide solutions
for all crazy scenarios...

Note that for critical things (for example things important for system
boot, etc.) there should be always hardcoded fallback.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: question about hardcoded binary paths (swapon / mkswap)

[Mike Frysinger]

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

On 01 Apr 2015 23:38, Karel Zak wrote:
> On Wed, Apr 01, 2015 at 10:06:52PM +0100, Ruediger Meier wrote:
> > > > Maybe both cases also with or without fallback $sbindir, /sbin or
> > > > $PATH.
> > > >
> > > > I guess we should agree how somthing like this should be handeled
> > > > in general. "eject" is also using hardcoded "/bin/umount".
> > >
> > > seems like $PATH should always be used.  if you broke $PATH, well
> 
> Yes, agree.
>  
> Note that we already have and use FS_SEARCH_PATH in mkfs, fsck and
> mount (libmount), see --enable-fs-paths-default and  --enable-fs-paths-extra.

what's the reason for having FS_SEARCH_PATH anymore ?  neither tool is set*id, 
and mkfs/fsck generally live in /sbin.  i guess if you're non-root and have 
/sbin/mkfs hardcoded in a script, then dropping FS_SEARCH_PATH might break 
existing code.

looking a bit at the code, i see that --disable-fs-paths-default almost does the 
right thing.  but the actual implementations are inconsistent leading to 
weirdness.

fsck adds / to the search:
...
static const char fsck_prefix_path[] = FS_SEARCH_PATH;
...
    char *oldpath = getenv("PATH");
...
    if (oldpath) {
        fsck_path = xmalloc (strlen (fsck_prefix_path) + 1 +
                    strlen (oldpath) + 1);
        strcpy (fsck_path, fsck_prefix_path);
        strcat (fsck_path, ":");
        strcat (fsck_path, oldpath);
...
    tpl = (strncmp(type, "fsck.", 5) ? "%s/fsck.%s" : "%s/%s");

    for(s = strtok(p, ":"); s; s = strtok(NULL, ":")) {
        sprintf(prog, tpl, s, type);
        if (stat(prog, &st) == 0)
            break;
    }
...

mkfs adds the cwd to $PATH, and hardcodes /bin too:
...
#define SEARCH_PATH "PATH=" FS_SEARCH_PATH
...
    /* Set PATH and program name */
    oldpath = getenv("PATH");
    if (!oldpath)
        oldpath = "/bin";

    newpath = xmalloc(strlen(oldpath) + sizeof(SEARCH_PATH) + 3);
    sprintf(newpath, "%s:%s\n", SEARCH_PATH, oldpath);
    putenv(newpath);
...

libmount only searches FS_SEARCH_PATH:
...
    char search_path[] = FS_SEARCH_PATH;        /* from config.h */
...
    path = strtok_r(search_path, ":", &p);
    while (path) {
...

> Maybe we can use it use FS_SEARCH_PATH also for mkswap in swapon, or use it
> as fallback.

my preference would be to not move more tools into that system and allow any 
more implicit lookups to leak out.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: question about hardcoded binary paths (swapon / mkswap)

[Karel Zak]

On Wed, Apr 01, 2015 at 09:12:30PM -0400, Mike Frysinger wrote:
> On 01 Apr 2015 23:38, Karel Zak wrote:
> > On Wed, Apr 01, 2015 at 10:06:52PM +0100, Ruediger Meier wrote:
> > > > > Maybe both cases also with or without fallback $sbindir, /sbin or
> > > > > $PATH.
> > > > >
> > > > > I guess we should agree how somthing like this should be handeled
> > > > > in general. "eject" is also using hardcoded "/bin/umount".
> > > >
> > > > seems like $PATH should always be used.  if you broke $PATH, well
> > 
> > Yes, agree.
> >  
> > Note that we already have and use FS_SEARCH_PATH in mkfs, fsck and
> > mount (libmount), see --enable-fs-paths-default and  --enable-fs-paths-extra.
> 
> what's the reason for having FS_SEARCH_PATH anymore ? 

If I good remember then the reason is that the helpers does not have
to be installed in standard PATH. Well, you're author of this thing
:-)

        commit bb4cb69df2a7fba3098f073aa4b758a8011d826f
        Author: Mike Frysinger <vapier@gentoo.org>
        Date:   Sun Jan 24 22:36:55 2010 -0500

        fsck/mkfs/mount: unify default search paths for helpers


> neither tool is set*id, 
> and mkfs/fsck generally live in /sbin.  i guess if you're non-root and have 
> /sbin/mkfs hardcoded in a script, then dropping FS_SEARCH_PATH might break 
> existing code.

for systemd based distors the path should be also modified, we have
all in /usr and /sbin and /bin are symlinks only.

> looking a bit at the code, i see that --disable-fs-paths-default almost does the 
> right thing.  but the actual implementations are inconsistent leading to 
> weirdness.
> 
> fsck adds / to the search:
> ...
> static const char fsck_prefix_path[] = FS_SEARCH_PATH;
> ...
>     char *oldpath = getenv("PATH");
> ...
>     if (oldpath) {
>         fsck_path = xmalloc (strlen (fsck_prefix_path) + 1 +
>                     strlen (oldpath) + 1);
>         strcpy (fsck_path, fsck_prefix_path);
>         strcat (fsck_path, ":");
>         strcat (fsck_path, oldpath);
> ...
>     tpl = (strncmp(type, "fsck.", 5) ? "%s/fsck.%s" : "%s/%s");
> 
>     for(s = strtok(p, ":"); s; s = strtok(NULL, ":")) {
>         sprintf(prog, tpl, s, type);
>         if (stat(prog, &st) == 0)
>             break;
>     }
> ...
> 
> mkfs adds the cwd to $PATH, and hardcodes /bin too:
> ...
> #define SEARCH_PATH "PATH=" FS_SEARCH_PATH
> ...
>     /* Set PATH and program name */
>     oldpath = getenv("PATH");
>     if (!oldpath)
>         oldpath = "/bin";
> 
>     newpath = xmalloc(strlen(oldpath) + sizeof(SEARCH_PATH) + 3);
>     sprintf(newpath, "%s:%s\n", SEARCH_PATH, oldpath);
>     putenv(newpath);
> ...
> 
> libmount only searches FS_SEARCH_PATH:
> ...
>     char search_path[] = FS_SEARCH_PATH;        /* from config.h */
> ...
>     path = strtok_r(search_path, ":", &p);
>     while (path) {
> ...
> 
> > Maybe we can use it use FS_SEARCH_PATH also for mkswap in swapon, or use it
> > as fallback.
> 
> my preference would be to not move more tools into that system and allow any 
> more implicit lookups to leak out.

I'm happy that for example mount(8) does not waste time with all PATH,
but it cares about /sbin only. IMHO it's fine that mkfs, mount and
fsck assume *helpers* on specific place. The mkswap is different, it's 
standard command and it's expected in PATH.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: question about hardcoded binary paths (swapon / mkswap)

[Mike Frysinger]

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

On 02 Apr 2015 10:20, Karel Zak wrote:
> On Wed, Apr 01, 2015 at 09:12:30PM -0400, Mike Frysinger wrote:
> > On 01 Apr 2015 23:38, Karel Zak wrote:
> > > On Wed, Apr 01, 2015 at 10:06:52PM +0100, Ruediger Meier wrote:
> > > > > > Maybe both cases also with or without fallback $sbindir, /sbin or
> > > > > > $PATH.
> > > > > >
> > > > > > I guess we should agree how somthing like this should be handeled
> > > > > > in general. "eject" is also using hardcoded "/bin/umount".
> > > > >
> > > > > seems like $PATH should always be used.  if you broke $PATH, well
> > > 
> > > Yes, agree.
> > >  
> > > Note that we already have and use FS_SEARCH_PATH in mkfs, fsck and
> > > mount (libmount), see --enable-fs-paths-default and  --enable-fs-paths-extra.
> > 
> > what's the reason for having FS_SEARCH_PATH anymore ? 
> 
> If I good remember then the reason is that the helpers does not have
> to be installed in standard PATH. Well, you're author of this thing
> :-)

i wrote the code to make it a configure option, but the actual behavior predates 
me.  i'm interested more in the behavior, not the exact configure option.

looks like mkfs added it during the 2.2->2.5 transition, but otherwise no 
details in the bundled NEWS that i saw.  oh well.

> > neither tool is set*id, 
> > and mkfs/fsck generally live in /sbin.  i guess if you're non-root and have 
> > /sbin/mkfs hardcoded in a script, then dropping FS_SEARCH_PATH might break 
> > existing code.
> 
> for systemd based distors the path should be also modified, we have
> all in /usr and /sbin and /bin are symlinks only.

but they'll still be in $PATH ?

> > > Maybe we can use it use FS_SEARCH_PATH also for mkswap in swapon, or use it
> > > as fallback.
> > 
> > my preference would be to not move more tools into that system and allow any 
> > more implicit lookups to leak out.
> 
> I'm happy that for example mount(8) does not waste time with all PATH,
> but it cares about /sbin only. IMHO it's fine that mkfs, mount and
> fsck assume *helpers* on specific place. The mkswap is different, it's 
> standard command and it's expected in PATH.

mount makes sense as it's set*id and we can't trust users to not be evil :).
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: question about hardcoded binary paths (swapon / mkswap)

[Karel Zak]

On Thu, Apr 02, 2015 at 12:19:52PM -0400, Mike Frysinger wrote:
> On 02 Apr 2015 10:20, Karel Zak wrote:
> > If I good remember then the reason is that the helpers does not have
> > to be installed in standard PATH. Well, you're author of this thing
> > :-)
> 
> i wrote the code to make it a configure option, but the actual behavior predates 
> me.  i'm interested more in the behavior, not the exact configure option.

 So, the basis question is if we really need to support non-standard
 paths for the helpers. IMHO it's unnecessary legacy and I don't see a
 problem to drop this feature and require $PATH, and for critical
 things like fsck fallback to /sbin if $PATH is undefined.

 Comments?

> looks like mkfs added it during the 2.2->2.5 transition, but otherwise no 
> details in the bundled NEWS that i saw.  oh well.

 mkfs is deprecated, the right way is to call directly mkfs.<type>.

> mount makes sense as it's set*id and we can't trust users to not be evil :)

 It does not execute anything with root rights, but yes, hardcoded
 paths make sense there (just to avoid complexity and external
 dependencies on environment).

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: question about hardcoded binary paths (swapon / mkswap)

[Ruediger Meier]

On Thursday 02 April 2015, Karel Zak wrote:
> On Thu, Apr 02, 2015 at 12:19:52PM -0400, Mike Frysinger wrote:
> > On 02 Apr 2015 10:20, Karel Zak wrote:
> > > If I good remember then the reason is that the helpers does not
> > > have to be installed in standard PATH. Well, you're author of
> > > this thing
> > >
> > > :-)
> >
> > i wrote the code to make it a configure option, but the actual
> > behavior predates me.  i'm interested more in the behavior, not the
> > exact configure option.
>
>  So, the basis question is if we really need to support non-standard
>  paths for the helpers. IMHO it's unnecessary legacy and I don't see
> a problem to drop this feature and require $PATH, and for critical
> things like fsck fallback to /sbin if $PATH is undefined.
>
>  Comments?

I would like to use PATH too but also fallback if PATH is defined. That 
means we just need to append our fallback path to PATH always.

The fallback path(s) may depend on what we are looking for.

In case we search an ul binary we should add the conigured and the 
default path (which is just one path in default case):

"$PATH:$sbindir:/sbin"   for sbin_PROGRAMS like "fsck" or "mkswap"
"$PATH:$bindir:/bin"     for bin_PROGRAMS like "mount/umount"
"$PATH:$usrsbin_execdir:/usr/sbin"  for usrsbin_exec_PROGRAMS
"$PATH:$usrbin_execdir:/usr/bin"    for usrbin_exec_PROGRAMS

For non-ul programs we should not add any fallback path unless there is 
a POSIX or other well-known one. "/bin/sh" might be the only real 
special case.

> > looks like mkfs added it during the 2.2->2.5 transition, but
> > otherwise no details in the bundled NEWS that i saw.  oh well.
>
>  mkfs is deprecated, the right way is to call directly mkfs.<type>.
>
> > mount makes sense as it's set*id and we can't trust users to not be
> > evil :)
>
>  It does not execute anything with root rights, but yes, hardcoded
>  paths make sense there (just to avoid complexity and external
>  dependencies on environment).

Even for mount I would like "$PATH:$bindir:/bin" too. If PATH is really 
a security issue then at least "$bindir:/bin". configure --prefix=xyz 
should install consistent usable util-linux progs.

cu,
Rudi

Re: question about hardcoded binary paths (swapon / mkswap)

[Mike Frysinger]

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

On 02 Apr 2015 21:15, Karel Zak wrote:
> On Thu, Apr 02, 2015 at 12:19:52PM -0400, Mike Frysinger wrote:
> > On 02 Apr 2015 10:20, Karel Zak wrote:
> > > If I good remember then the reason is that the helpers does not have
> > > to be installed in standard PATH. Well, you're author of this thing
> > > :-)
> > 
> > i wrote the code to make it a configure option, but the actual behavior predates 
> > me.  i'm interested more in the behavior, not the exact configure option.
> 
>  So, the basis question is if we really need to support non-standard
>  paths for the helpers. IMHO it's unnecessary legacy and I don't see a
>  problem to drop this feature and require $PATH, and for critical
>  things like fsck fallback to /sbin if $PATH is undefined.

the reason for adding that configure option was to support packages that install 
both into /bin and /usr/bin.  i understand some distros will override those 
settings of upstream packages, but Gentoo has opted not to since there's no 
reason at all to force them all into /sbin (and even existing tools in /sbin are 
pretty pointless).  although it mattered more when the code was only searching 
that list and not $PATH at all.

my preference would be just to do execvp() and be done so we can stop these 
distro bikesheddings (/bin & /usr-merge and such).
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: question about hardcoded binary paths (swapon / mkswap)

[Karel Zak]

On Thu, Apr 02, 2015 at 09:15:22PM -0400, Mike Frysinger wrote:
> On 02 Apr 2015 21:15, Karel Zak wrote:
> > On Thu, Apr 02, 2015 at 12:19:52PM -0400, Mike Frysinger wrote:
> > > On 02 Apr 2015 10:20, Karel Zak wrote:
> > > > If I good remember then the reason is that the helpers does not have
> > > > to be installed in standard PATH. Well, you're author of this thing
> > > > :-)
> > > 
> > > i wrote the code to make it a configure option, but the actual behavior predates 
> > > me.  i'm interested more in the behavior, not the exact configure option.
> > 
> >  So, the basis question is if we really need to support non-standard
> >  paths for the helpers. IMHO it's unnecessary legacy and I don't see a
> >  problem to drop this feature and require $PATH, and for critical
> >  things like fsck fallback to /sbin if $PATH is undefined.
> 
> the reason for adding that configure option was to support packages that install 
> both into /bin and /usr/bin.  i understand some distros will override those 

I have talked about crazy things like /sbin/fs.d or /sbin/fs. The standard
paths like [/usr]/bin, [/usr]/sbin are not problem.

> settings of upstream packages, but Gentoo has opted not to since there's no 
> reason at all to force them all into /sbin (and even existing tools in /sbin are 
> pretty pointless).  although it mattered more when the code was only searching 
> that list and not $PATH at all.
> 
> my preference would be just to do execvp() and be done so we can stop these 
> distro bikesheddings (/bin & /usr-merge and such).

Yes, I thought about it too... just move the problem with PATH to libc ;-)

For example mkfs already uses execvp(), I guess we can do the same in
fsck. For mount(8) it would be better to follow the current behaviour, 
but remove nonsenses from FS_SEARCH_PATH (and rename to LIBMOUNT_HELPERS_PATH).

Volunteers? ;-)

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: question about hardcoded binary paths (swapon / mkswap)

[Mike Frysinger]

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

On 03 Apr 2015 10:52, Karel Zak wrote:
> On Thu, Apr 02, 2015 at 09:15:22PM -0400, Mike Frysinger wrote:
> > On 02 Apr 2015 21:15, Karel Zak wrote:
> > > On Thu, Apr 02, 2015 at 12:19:52PM -0400, Mike Frysinger wrote:
> > > > On 02 Apr 2015 10:20, Karel Zak wrote:
> > > > > If I good remember then the reason is that the helpers does not have
> > > > > to be installed in standard PATH. Well, you're author of this thing
> > > > > :-)
> > > > 
> > > > i wrote the code to make it a configure option, but the actual behavior predates 
> > > > me.  i'm interested more in the behavior, not the exact configure option.
> > > 
> > >  So, the basis question is if we really need to support non-standard
> > >  paths for the helpers. IMHO it's unnecessary legacy and I don't see a
> > >  problem to drop this feature and require $PATH, and for critical
> > >  things like fsck fallback to /sbin if $PATH is undefined.
> > 
> > the reason for adding that configure option was to support packages that install 
> > both into /bin and /usr/bin.  i understand some distros will override those 
> 
> I have talked about crazy things like /sbin/fs.d or /sbin/fs. The standard
> paths like [/usr]/bin, [/usr]/sbin are not problem.

i have no problem with punting fd.d & fs subdirs.  i don't recall ever seeing a 
Linux system use them, although i've only been running things since the 2.4 
days, so maybe i'm not old enough.

> > settings of upstream packages, but Gentoo has opted not to since there's no 
> > reason at all to force them all into /sbin (and even existing tools in /sbin are 
> > pretty pointless).  although it mattered more when the code was only searching 
> > that list and not $PATH at all.
> > 
> > my preference would be just to do execvp() and be done so we can stop these 
> > distro bikesheddings (/bin & /usr-merge and such).
> 
> Yes, I thought about it too... just move the problem with PATH to libc ;-)
> 
> For example mkfs already uses execvp(), I guess we can do the same in
> fsck. For mount(8) it would be better to follow the current behaviour, 
> but remove nonsenses from FS_SEARCH_PATH (and rename to LIBMOUNT_HELPERS_PATH).
> 
> Volunteers? ;-)

i'll take a stab at mkfs first
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: question about hardcoded binary paths (swapon / mkswap)

[Isaac Dunham]

On Thu, Apr 02, 2015 at 10:20:00AM +0200, Karel Zak wrote:
> On Wed, Apr 01, 2015 at 09:12:30PM -0400, Mike Frysinger wrote:
> > neither tool is set*id, 
> > and mkfs/fsck generally live in /sbin.  i guess if you're non-root and have 
> > /sbin/mkfs hardcoded in a script, then dropping FS_SEARCH_PATH might break 
> > existing code.
> 
> for systemd based distors the path should be also modified, we have
> all in /usr and /sbin and /bin are symlinks only.

(1) mkfs/fsck can be used on files that can be loopback mounted; I've 
used mkfs.*fs directly, but it may be desireable to support using 
mkfs -t as a user for creating filesystems on regular files.

(2) the /usr merge may be prevalent on RPM-based distros, but Debian does
not use it; I would assume that if they decided to support it, there would
be 2 releases before you could require it.
Additionally, the /sbin and /bin symlinks are there precisely because the
LSB requires certain commands to be in /bin and /sbin so that binaries
and scripts can rely on the paths. Ignoring this will cause needless
incompatability.

Thanks,
Isaac Dunham

Re: question about hardcoded binary paths (swapon / mkswap)

[Mike Frysinger]

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

On 01 Apr 2015 22:06, Ruediger Meier wrote:
> On Wednesday 01 April 2015, Mike Frysinger wrote:
> > On 01 Apr 2015 18:17, Ruediger Meier wrote:
> > > On Wednesday 01 April 2015, Isaac Dunham wrote:
> > > > On Wed, Apr 01, 2015 at 01:42:56PM +0200, Ruediger Meier wrote:
> > > > > I wonder about some hardcoded binary paths.
> > > > >
> > > > > Example swapon.c:
> > > > >
> > > > > #define PATH_MKSWAP    "/sbin/mkswap"
> > > > >
> > > > > There are a two problems.
> > > > > 1. It's wrong. We should use $sbindir from configure.
> > > > > 2. When called from our test-suite it will use a wrong (or
> > > > >    non-existend, broken) binary. This happens in test
> > > > > swapon/fixpgsz.
> > > > >
> > > > > The question is how to fix this.
> > > > >
> > > > > I would prefer to use "mkwsap" from the same directory like
> > > > > swapon or to simply execvp "mkswap" from PATH. But don't know
> > > > > if we want this. If we really want to keep a hardcoded sbindir
> > > > > then we would need "#ifdef TEST_PROGRAM".
> > > > >
> > > > > Any comments?
> > > >
> > > > The approach that seems obvious to me (assuming you want to keep
> > > > the hardcoded path) is:
> > > > -add -DSBINDIR="$sbindir" to CFLAGS
> > >
> > > Yes, this would be easy. But my preferred logic would be like this
> > >
> > > If "swapon" was called from PATH then just take mkswap from PATH
> > > too. If "/whatever/path/swapon" was called then look for mkswap in
> > > the same path.
> > >
> > > Maybe both cases also with or without fallback $sbindir, /sbin or
> > > $PATH.
> > >
> > > I guess we should agree how somthing like this should be handeled
> > > in general. "eject" is also using hardcoded "/bin/umount".
> >
> > seems like $PATH should always be used.  if you broke $PATH, well
> > that's your fualt ... tools shouldn't generally be expected to work
> > in the fact of hostile environments like this.
> > -mike
> 
> I'd also like to use PATH (at least as fallback).
> 
> The only special thing here is IMO that mkswap and swapon belong to the 
> same project. Should we try to use the right one per default?
> 
> Example:
> UL is installed below /usr/local
> PATH="/usr/local/sbin:/sbin"
> 
> Explicitly invoking the (old) globally installed /sbin/swapon should use 
> the old /sbin/mkswap too or the new /usr/local/sbin/mkswap from PATH? 
> 
> Another point:
> If "/sbin" is not in PATH should "sudo /sbin/swapon" find /sbin/mkswap 
> or not?

imo it should just use the $PATH and be done.  sudo will set up a sane path for 
you which includes /sbin.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]