conf/164048: /etc/rc.d/hostid is not symlink aware
Matthew Story
matthewstory at gmail.com
Thu Jan 12 16:50:12 UTC 2012
The following reply was made to PR conf/164048; it has been noted by GNATS.
From: Matthew Story <matthewstory at gmail.com>
To: Dirk-Willem van Gulik <dirkx at webweaving.org>
Cc: freebsd-gnats-submit at freebsd.org, freebsd-bugs at freebsd.org
Subject: Re: conf/164048: /etc/rc.d/hostid is not symlink aware
Date: Thu, 12 Jan 2012 11:48:48 -0500
--20cf307abe6dba156b04b65785f6
Content-Type: text/plain; charset=ISO-8859-1
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
--20cf307abe6dba156b04b65785f6
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
On Thu, Jan 12, 2012 at 3:15 AM, Dirk-Willem van Gulik <span dir=3D"ltr">&l=
t;<a href=3D"mailto:dirkx at webweaving.org">dirkx at webweaving.org</a>></spa=
n> wrote:=A0<div>[...snip]=A0<div class=3D"gmail_quote"><blockquote class=
=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padd=
ing-left:1ex">
=A0 =A0 =A0 # If ${hostid_file} already exists, we take UUID from there.<b=
r>
- =A0 =A0 =A0 if [ -r ${hostid_file} ]; then<br>
+ =A0 =A0 =A0 # If ${hostid_file} already exists, we take UUID from there. =
We use<br>
+ =A0 =A0 =A0 # a -f rather than a -r check as the histid_file may in fact =
be<br>
+ =A0 =A0 =A0 # a symbolic link.<br></blockquote><div><br></div><div>per th=
e test man-page, `-r' tests for readability, regardless of type, and `-=
f' tests for the existence of a regular file. =A0`-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):</div>
<div><br></div><div>$ # setup target/src dirs for demonstration of test</di=
v><div>$ mkdir target src</div><div>$ # setup source_files for sym-linking<=
/div><div>$ jot - 1 10 | xargs -I {} touch src/{}</div><div>$ # symlink in =
files</div>
<div>$ find "$PWD/src" -type f -depth 1 -maxdepth 1 -print0 | xar=
gs -0 -n1 sh -c 'ln -s "$*" target/' worker</div><div>$ #=
make target read-only, note that mode 0400 on target will result in -r to =
fail</div>
<div>$ chmod 500 target</div><div>$ # demonstrate that -r works with symlin=
ks</div><div>$ jot - 1 10 | while read trg; do [ -r "target/$trg"=
] && echo "I can read target/$trg"; done</div><div>I can=
read target/1</div>
<div>I can read target/2</div><div>I can read target/3</div><div>I can read=
target/4</div><div>I can read target/5</div><div>I can read target/6</div>=
<div>I can read target/7</div><div>I can read target/8</div><div>I can read=
target/9</div>
<div>I can read target/10</div><div>$ # demonstrate that -f also works with=
symlinks</div><div>$=A0=A0jot - 1 10 | while read trg; do [ -f "targe=
t/$trg" ] && echo "target/$trg is a regular file"; d=
one</div>
<div>target/1 is a regular file</div><div>target/2 is a regular file</div><=
div>target/3 is a regular file</div><div>target/4 is a regular file</div><d=
iv>target/5 is a regular file</div><div>target/6 is a regular file</div>
<div>target/7 is a regular file</div><div>target/8 is a regular file</div><=
div>target/9 is a regular file</div><div>target/10 is a regular file</div><=
div><br></div><div>=A0</div><blockquote class=3D"gmail_quote" style=3D"marg=
in:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ =A0 =A0 =A0 #<br>
+ =A0 =A0 =A0 if [ -f ${hostid_file} ]; then<br>
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hostid_set `cat ${hostid_file}`<br></blockq=
uote><div><br></div><div>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). =A0this is not the d=
esired behavior:</div>
<div><br></div><div>$ # using our above setup, make target/1 unreadable</di=
v><div><div>$ chmod 000 target/1</div><div>$ # demonstrate failure of the n=
ew test with an unreadable, but existing file</div><div>$ [ -f target/1 ] &=
amp;& cat target/1</div>
<div>cat: target/1: Permission denied</div></div><div>=A0</div><blockquote =
class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid=
;padding-left:1ex">
=A0 =A0 =A0 =A0else<br>
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0# No hostid file, generate UUID.<br>
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 hostid_generate<br>
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 hostid_reset<br></blockquote><div><br></div><=
div>This line is actually why you are seeing a hostid_file on restart. =A0T=
he hostid_file does not exist on your system, and per the comment, and impl=
ementation, if a hostid_file does not exist, one is generated and set via s=
ysctl (via the hostid_set function).</div>
<div><br></div><div>Your suggested change changes the functionality of this=
program to both generate a hostid, and then store it in a hostid file. =A0=
This seems to me to be a change in functionality, and not a bug.</div><div>
=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;borde=
r-left:1px #ccc solid;padding-left:1ex">=A0[...snip]<br></blockquote><div><=
br></div><div>There is a small race condition in this file (unless rc.d is =
doing some locking on hostid_file in the caller)</div>
<div><br></div><div>if [ -r ${hostid_file} ]; then</div><div>=A0=A0 =A0host=
id_set `cat ${hostid_file}`</div><div>else=A0</div></div>=A0=A0 =A0...</div=
><div><br></div><div>Insofar as it's possible (however unlikely) that t=
he file mode (or file mode of the parents) could change between the test an=
d the read. =A0This could probably be resolved using lockf, but it's de=
finitely not a big deal.</div>
<div><br></div><div>---------------snippits from man 1 test----------------=
-</div><div><br></div><div>=A0=A0 =A0 -r file =A0 =A0 =A0 True if file exis=
ts and is readable.</div><div>=A0=A0 =A0 [...snip]</div><div>=A0=A0 =A0 -f =
file =A0 =A0 =A0 True if file exists and is a regular file.</div>
<div><br></div><div><br clear=3D"all"><div><br></div>-- <br>regards,<br>mat=
t<br>
</div>
--20cf307abe6dba156b04b65785f6--
More information about the freebsd-bugs
mailing list