Re: [Qemu-block] [PATCH 1/7] qemu-iotests: check: don't place files with predictable names in /tmp

[Sascha Silbe]

Dear Max,

Max Reitz  writes:

> You decide whether you want to drop the tmp=/tmp/$$ lines in the tests
> in a dedicated (follow-up) patch or include it here.

Let's land this one first; I'll address the other occurrences in a
separate patch. I've put it on my todo list.

Thanks for the reviews!

Sascha
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641

Re: [Qemu-block] [PATCH 1/7] qemu-iotests: check: don't place files with predictable names in /tmp

[Max Reitz]

On 05.04.2016 11:21, Sascha Silbe wrote:
> Placing files with predictable or even hard-coded names in /tmp is a
> security risk and can prevent or disturb operation on a multi-user
> machine. Place them inside the "scratch" directory instead, as we
> already do for most other test-related files.
> 
> Signed-off-by: Sascha Silbe 
> Reviewed-by: Bo Tu 
> ---
>  tests/qemu-iotests/check | 21 +++--
>  1 file changed, 11 insertions(+), 10 deletions(-)

Nice side effect: With this patch it's possible to run multiple
instances of the iotests in parallel (for different formats/protocols)
without them interfering with each other.

Grepping for '/tmp' in the iotests directory yields more occurrences,
however: Many tests set the tmp variable to /tmp/$$. Let's see whether
we can just remove that or have to replace it by "${TEST_DIR}"/$$.

"common.filter" evaluates $tmp, but the single filter that does so is
actually never used any more. Other than that, only "common" evaluates
it, but "common" is sourced by "check". Thus I think those tests setting
$tmp is superfluous and dropping it should be fine.

For this patch:

Reviewed-by: Max Reitz 

You decide whether you want to drop the tmp=/tmp/$$ lines in the tests
in a dedicated (follow-up) patch or include it here.

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-block] [PATCH 1/7] qemu-iotests: check: don't place files with predictable names in /tmp

[Sascha Silbe]

Placing files with predictable or even hard-coded names in /tmp is a
security risk and can prevent or disturb operation on a multi-user
machine. Place them inside the "scratch" directory instead, as we
already do for most other test-related files.

Signed-off-by: Sascha Silbe 
Reviewed-by: Bo Tu 
---
 tests/qemu-iotests/check | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index c350f16..4cba215 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -19,7 +19,6 @@
 # Control script for QA
 #
 
-tmp=/tmp/$$
 status=0
 needwrap=true
 try=0
@@ -130,6 +129,8 @@ fi
 #exit 1
 #fi
 
+tmp="${TEST_DIR}"/$$
+
 _wallclock()
 {
 date "+%H %M %S" | $AWK_PROG '{ print $1*3600 + $2*60 + $3 }'
@@ -146,8 +147,8 @@ _wrapup()
 # for hangcheck ...
 # remove files that were used by hangcheck
 #
-[ -f /tmp/check.pid ] && rm -rf /tmp/check.pid
-[ -f /tmp/check.sts ] && rm -rf /tmp/check.sts
+[ -f "${TEST_DIR}"/check.pid ] && rm -rf "${TEST_DIR}"/check.pid
+[ -f "${TEST_DIR}"/check.sts ] && rm -rf "${TEST_DIR}"/check.sts
 
 if $showme
 then
@@ -197,8 +198,8 @@ END{ if (NR > 0) {
 needwrap=false
 fi
 
-rm -f /tmp/*.out /tmp/*.err /tmp/*.time
-rm -f /tmp/check.pid /tmp/check.sts
+rm -f "${TEST_DIR}"/*.out "${TEST_DIR}"/*.err "${TEST_DIR}"/*.time
+rm -f "${TEST_DIR}"/check.pid "${TEST_DIR}"/check.sts
 rm -f $tmp.*
 }
 
@@ -208,16 +209,16 @@ trap "_wrapup; exit \$status" 0 1 2 3 15
 # Save pid of check in a well known place, so that hangcheck can be sure it
 # has the right pid (getting the pid from ps output is not reliable enough).
 #
-rm -rf /tmp/check.pid
-echo $$ >/tmp/check.pid
+rm -rf "${TEST_DIR}"/check.pid
+echo $$ > "${TEST_DIR}"/check.pid
 
 # for hangcheck ...
 # Save the status of check in a well known place, so that hangcheck can be
 # sure to know where check is up to (getting test number from ps output is
 # not reliable enough since the trace stuff has been introduced).
 #
-rm -rf /tmp/check.sts
-echo "preamble" >/tmp/check.sts
+rm -rf "${TEST_DIR}"/check.sts
+echo "preamble" > "${TEST_DIR}"/check.sts
 
 # don't leave old full output behind on a clean run
 rm -f check.full
@@ -285,7 +286,7 @@ do
 rm -f core $seq.notrun
 
 # for hangcheck ...
-echo "$seq" >/tmp/check.sts
+echo "$seq" > "${TEST_DIR}"/check.sts
 
 start=`_wallclock`
 $timestamp && echo -n "["`date "+%T"`"]"
-- 
1.9.1