conf/164048: /etc/rc.d/hostid is not symlink aware

[Matthew Story]

On Thu, Jan 12, 2012 at 3:15 AM, Dirk-Willem van Gulik <dirkx at webweaving.org
> wrote:
[...snip]

>       # If ${hostid_file} already exists, we take UUID from there.
> -       if [ -r ${hostid_file} ]; then
> +       # If ${hostid_file} already exists, we take UUID from there. We use
> +       # a -f rather than a -r check as the histid_file may in fact be
> +       # a symbolic link.
>

per the test man-page, `-r' tests for readability, regardless of type, and
`-f' tests for the existence of a regular file.  `-r' does include an
implicit test for existence, so `-r' will in fact work for symlinks, and
fail reliably if the symlink source_file does not exist (relevant bits from
the test man-page at the bottom of this message):

$ # setup target/src dirs for demonstration of test
$ mkdir target src
$ # setup source_files for sym-linking
$ jot - 1 10 | xargs -I {} touch src/{}
$ # symlink in files
$ find "$PWD/src" -type f -depth 1 -maxdepth 1 -print0 | xargs -0 -n1 sh -c
'ln -s "$*" target/' worker
$ # make target read-only, note that mode 0400 on target will result in -r
to fail
$ chmod 500 target
$ # demonstrate that -r works with symlinks
$ jot - 1 10 | while read trg; do [ -r "target/$trg" ] && echo "I can read
target/$trg"; done
I can read target/1
I can read target/2
I can read target/3
I can read target/4
I can read target/5
I can read target/6
I can read target/7
I can read target/8
I can read target/9
I can read target/10
$ # demonstrate that -f also works with symlinks
$  jot - 1 10 | while read trg; do [ -f "target/$trg" ] && echo
"target/$trg is a regular file"; done
target/1 is a regular file
target/2 is a regular file
target/3 is a regular file
target/4 is a regular file
target/5 is a regular file
target/6 is a regular file
target/7 is a regular file
target/8 is a regular file
target/9 is a regular file
target/10 is a regular file



> +       #
> +       if [ -f ${hostid_file} ]; then
>                hostid_set `cat ${hostid_file}`
>

with this patch, if ${hostid_file} exists, and is non-readable, cat
${hostid_file} will fail, and yield no $1 to hostid_set (effectively
identical to a hostid_file that is empty).  this is not the desired
behavior:

$ # using our above setup, make target/1 unreadable
$ chmod 000 target/1
$ # demonstrate failure of the new test with an unreadable, but existing
file
$ [ -f target/1 ] && cat target/1
cat: target/1: Permission denied


>        else
>                # No hostid file, generate UUID.
> -               hostid_generate
> +               hostid_reset
>

This line is actually why you are seeing a hostid_file on restart.  The
hostid_file does not exist on your system, and per the comment, and
implementation, if a hostid_file does not exist, one is generated and set
via sysctl (via the hostid_set function).

Your suggested change changes the functionality of this program to both
generate a hostid, and then store it in a hostid file.  This seems to me to
be a change in functionality, and not a bug.


>  [...snip]
>

There is a small race condition in this file (unless rc.d is doing some
locking on hostid_file in the caller)

if [ -r ${hostid_file} ]; then
    hostid_set `cat ${hostid_file}`
else
    ...

Insofar as it's possible (however unlikely) that the file mode (or file
mode of the parents) could change between the test and the read.  This
could probably be resolved using lockf, but it's definitely not a big deal.

---------------snippits from man 1 test-----------------

     -r file       True if file exists and is readable.
     [...snip]
     -f file       True if file exists and is a regular file.



-- 
regards,
matt