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

[Mike Frysinger <vapier@gentoo.org>]

[-- 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 --]