Re: install-sh and $RANDOM
On 10/17/2016 11:46 PM, co...@sdf.org wrote:
> On Mon, Oct 17, 2016 at 04:56:05PM -0500, Eric Blake wrote:
>> Second, your claim that things are "spuriously bad if $RANDOM does not
>> exist" is false. Look at the full context:
>>
>> tmpdir=${TMPDIR-/tmp}/ins$RANDOM-$$
>> trap 'ret=$?; rmdir "$tmpdir/d" "$tmpdir" 2>/dev/null; exit
>> $ret' 0
>
> I don't mean that it's dangerous to use (endangers the user), but deleting
> those directories when $tmpdir is just /tmp/ins- will make this script race
> other instances of itself, and delete their work.
Except that it won't be just /tmp/ins-, but /tmp/ins-$$ (that is, the
pid is encoded into each directory); parallel runs of this script have
different pids and thus different directories.
>
> (I didn't find the script in automake's git repository, so assumed this
> is the place)
automake.git/lib/install-sh
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: install-sh and $RANDOM
On Tue, Oct 18, 2016 at 06:24:03AM -0500, Eric Blake wrote:
> On 10/17/2016 11:46 PM, co...@sdf.org wrote:
> > On Mon, Oct 17, 2016 at 04:56:05PM -0500, Eric Blake wrote:
> >> Second, your claim that things are "spuriously bad if $RANDOM does not
> >> exist" is false. Look at the full context:
> >>
> >> tmpdir=${TMPDIR-/tmp}/ins$RANDOM-$$
> >> trap 'ret=$?; rmdir "$tmpdir/d" "$tmpdir" 2>/dev/null; exit
> >> $ret' 0
> >
> > I don't mean that it's dangerous to use (endangers the user), but deleting
> > those directories when $tmpdir is just /tmp/ins- will make this script race
> > other instances of itself, and delete their work.
>
> Except that it won't be just /tmp/ins-, but /tmp/ins-$$ (that is, the
> pid is encoded into each directory); parallel runs of this script have
> different pids and thus different directories.
>
> >
> > (I didn't find the script in automake's git repository, so assumed this
> > is the place)
>
> automake.git/lib/install-sh
>
> --
> Eric Blake eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
Ah!
Sorry for the noise then.
Re: install-sh and $RANDOM
On Mon, Oct 17, 2016 at 04:56:05PM -0500, Eric Blake wrote:
> Second, your claim that things are "spuriously bad if $RANDOM does not
> exist" is false. Look at the full context:
>
> tmpdir=${TMPDIR-/tmp}/ins$RANDOM-$$
> trap 'ret=$?; rmdir "$tmpdir/d" "$tmpdir" 2>/dev/null; exit
> $ret' 0
I don't mean that it's dangerous to use (endangers the user), but deleting
those directories when $tmpdir is just /tmp/ins- will make this script race
other instances of itself, and delete their work.
(I didn't find the script in automake's git repository, so assumed this
is the place)
Re: install-sh and $RANDOM
On Mon, Oct 17, 2016 at 04:56:05PM -0500, Eric Blake wrote: > You're not the first person to complain that $RANDOM is a bashism, and > this is not the first time we've had to retort that our use of $RANDOM > is a nicety, but not a necessity, and that the code is perfectly safe > and tested on shells where the expansion of $RANDOM is the empty string. I can see why it would surprise people, so maybe it would be helpful to add a comment mentioning that the code is still safe when $RANDOM is not useful, and perhaps pointing to this discussion.
Re: install-sh and $RANDOM
On 10/17/2016 03:58 PM, co...@sdf.org wrote:
> Hi,
>
> in build-aux/install-sh scriptversion=2016-01-11.22 line 327
> tmpdir=${TMPDIR-/tmp}/ins$RANDOM-$$
> ... things which are spuriously bad if $RANDOM does not exist ...
Thanks for the report. However, you've made two mistakes.
First, gnulib does not maintain install-sh - that is automake's job, so
if anything needs to change, it would have to change upstream in
automake first.
Second, your claim that things are "spuriously bad if $RANDOM does not
exist" is false. Look at the full context:
tmpdir=${TMPDIR-/tmp}/ins$RANDOM-$$
trap 'ret=$?; rmdir "$tmpdir/d" "$tmpdir" 2>/dev/null; exit
$ret' 0
if (umask $mkdir_umask &&
exec $mkdirprog $mkdir_mode -p -- "$tmpdir/d")
>/dev/null 2>&1
then
if test -z "$dir_arg" || {
...
fi
rmdir "$tmpdir/d" "$tmpdir"
else
# Remove any dirs left behind by ancient mkdir
implementations.
rmdir ./$mkdir_mode ./-p ./-- 2>/dev/null
fi
trap '' 0;;
esac;;
esac
if
$posix_mkdir && (
umask $mkdir_umask &&
$doit_exec $mkdirprog $mkdir_mode -p -- "$dstdir"
)
then :
else
# The umask is ridiculous, or mkdir does not conform to POSIX,
# or it failed possibly due to a race condition. Create the
# directory the slow way, step by step, checking for races as we go.
Whether the directory is named '/tmp/ins1234-5678' (on bash) or
'/tmp/ins-5678' (on shells that lack $RANDOM), the remaining code is
STILL atomically correct - either the mkdir succeeds or fails, but there
is NO way that the script can be coerced into mistakenly acting on an
unintended file because someone was able to predict the directory name.
True, collisions are more likely on a setup without $RANDOM, but the
result of a collision is merely that the script gracefully falls back to
slower code, NOT that it operates unsafely on the colliding name.
You're not the first person to complain that $RANDOM is a bashism, and
this is not the first time we've had to retort that our use of $RANDOM
is a nicety, but not a necessity, and that the code is perfectly safe
and tested on shells where the expansion of $RANDOM is the empty string.
>
> Please use something like:
> tmpdir=$(mktemp -d -p ${TMPDIR:-/tmp})
>
> Note also :
>
> NetBSD /bin/sh does not have $RANDOM.
> I don't know how portable mktemp is. sorry.
Less portable than $RANDOM expanding to the empty string.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
install-sh and $RANDOM
Hi,
in build-aux/install-sh scriptversion=2016-01-11.22 line 327
tmpdir=${TMPDIR-/tmp}/ins$RANDOM-$$
... things which are spuriously bad if $RANDOM does not exist ...
Please use something like:
tmpdir=$(mktemp -d -p ${TMPDIR:-/tmp})
Note also :
NetBSD /bin/sh does not have $RANDOM.
I don't know how portable mktemp is. sorry.
Thanks.
