* [PATCH] fix(script): improve safety and formatting for WRITES_STRICT restore
@ 2025-04-23 8:32 Alexander Shatalin
2025-04-28 20:52 ` Shuah Khan
0 siblings, 1 reply; 2+ messages in thread
From: Alexander Shatalin @ 2025-04-23 8:32 UTC (permalink / raw)
To: linux-kselftest; +Cc: skhan
[-- Attachment #1.1: Type: text/plain, Size: 338 bytes --]
From: Alexander Shatalin <sashatalin03@gmail.com>
This patch improves the safety when restoring WRITES_STRICT by ensuring:
- The variable is not empty (`-n`)
- The target file is writable (`-w`)
Also improves formatting by quoting variables and following shell best
practices.
Signed-off-by: Alexander Shatalin <sashatalin03@gmail.com>
[-- Attachment #1.2: Type: text/html, Size: 485 bytes --]
[-- Attachment #2: 0001-selftests-sysctl-improve-safety-and-formatting-for-W.patch --]
[-- Type: application/octet-stream, Size: 15849 bytes --]
From 5a007a322593577eff8babbebf7ddfeecf122c01 Mon Sep 17 00:00:00 2001
From: Alexander Shatalin <sashatalin03@gmail.com>
Date: Wed, 23 Apr 2025 11:31:49 +0300
Subject: [PATCH] selftests/sysctl: improve safety and formatting for
WRITES_STRICT restore
Signed-off-by: Alexander Shatalin <sashatalin03@gmail.com>
---
tools/testing/selftests/sysctl/sysctl.sh | 254 ++++++++++-------------
1 file changed, 114 insertions(+), 140 deletions(-)
diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index f50778a3d744..23be6ec28807 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -42,8 +42,7 @@ ALL_TESTS="$ALL_TESTS 0006:50:1:bitmap_0001"
ALL_TESTS="$ALL_TESTS 0007:1:1:boot_int"
ALL_TESTS="$ALL_TESTS 0008:1:1:match_int"
-function allow_user_defaults()
-{
+function allow_user_defaults() {
if [ -z $DIR ]; then
DIR="/sys/module/test_sysctl/"
fi
@@ -61,8 +60,7 @@ function allow_user_defaults()
fi
}
-function check_production_sysctl_writes_strict()
-{
+function check_production_sysctl_writes_strict() {
echo -n "Checking production write strict setting ... "
if [ ! -e ${WRITES_STRICT} ]; then
echo "FAIL, but skip in case of old kernel" >&2
@@ -72,7 +70,7 @@ function check_production_sysctl_writes_strict()
echo "ok"
else
echo "FAIL, strict value is 0 but force to 1 to continue" >&2
- echo "1" > ${WRITES_STRICT}
+ echo "1" >${WRITES_STRICT}
fi
fi
@@ -80,7 +78,7 @@ function check_production_sysctl_writes_strict()
PAGE_SIZE=$(getconf PAGESIZE)
fi
if [ -z $MAX_DIGITS ]; then
- MAX_DIGITS=$(($PAGE_SIZE/8))
+ MAX_DIGITS=$(($PAGE_SIZE / 8))
fi
if [ -z $INT_MAX ]; then
INT_MAX=$(getconf INT_MAX)
@@ -90,30 +88,28 @@ function check_production_sysctl_writes_strict()
fi
}
-test_reqs()
-{
+test_reqs() {
uid=$(id -u)
if [ $uid -ne 0 ]; then
echo $msg must be run as root >&2
exit $ksft_skip
fi
- if ! which perl 2> /dev/null > /dev/null; then
+ if ! which perl 2>/dev/null >/dev/null; then
echo "$0: You need perl installed"
exit $ksft_skip
fi
- if ! which getconf 2> /dev/null > /dev/null; then
+ if ! which getconf 2>/dev/null >/dev/null; then
echo "$0: You need getconf installed"
exit $ksft_skip
fi
- if ! which diff 2> /dev/null > /dev/null; then
+ if ! which diff 2>/dev/null >/dev/null; then
echo "$0: You need diff installed"
exit $ksft_skip
fi
}
-function load_req_mod()
-{
+function load_req_mod() {
if [ ! -d $SYSCTL ]; then
if ! modprobe -q -n $TEST_DRIVER; then
echo "$0: module $TEST_DRIVER not found [SKIP]"
@@ -128,48 +124,43 @@ function load_req_mod()
fi
}
-reset_vals()
-{
+reset_vals() {
VAL=""
TRIGGER=$(basename ${TARGET})
case "$TRIGGER" in
- int_0001)
- VAL="60"
- ;;
- int_0002)
- VAL="1"
- ;;
- uint_0001)
- VAL="314"
- ;;
- string_0001)
- VAL="(none)"
- ;;
- bitmap_0001)
- VAL=""
- ;;
- *)
- ;;
+ int_0001)
+ VAL="60"
+ ;;
+ int_0002)
+ VAL="1"
+ ;;
+ uint_0001)
+ VAL="314"
+ ;;
+ string_0001)
+ VAL="(none)"
+ ;;
+ bitmap_0001)
+ VAL=""
+ ;;
+ *) ;;
esac
- echo -n $VAL > $TARGET
+ echo -n $VAL >$TARGET
}
-set_orig()
-{
+set_orig() {
if [ ! -z $TARGET ] && [ ! -z $ORIG ]; then
if [ -f ${TARGET} ]; then
- echo "${ORIG}" > "${TARGET}"
+ echo "${ORIG}" >"${TARGET}"
fi
fi
}
-set_test()
-{
- echo "${TEST_STR}" > "${TARGET}"
+set_test() {
+ echo "${TEST_STR}" >"${TARGET}"
}
-verify()
-{
+verify() {
local seen
seen=$(cat "$1")
if [ "${seen}" != "${TEST_STR}" ]; then
@@ -182,10 +173,9 @@ verify()
# and get you incorrect results on proc files with long data. To use
# diff against them you must first extract the output to a file, and
# then compare against that file.
-verify_diff_proc_file()
-{
+verify_diff_proc_file() {
TMP_DUMP_FILE=$(mktemp)
- cat $1 > $TMP_DUMP_FILE
+ cat $1 >$TMP_DUMP_FILE
if ! diff -w -q $TMP_DUMP_FILE $2; then
return 1
@@ -194,39 +184,35 @@ verify_diff_proc_file()
fi
}
-verify_diff_w()
-{
- echo "$TEST_STR" | diff -q -w -u - $1 > /dev/null
+verify_diff_w() {
+ echo "$TEST_STR" | diff -q -w -u - $1 >/dev/null
return $?
}
-test_rc()
-{
+test_rc() {
if [[ $rc != 0 ]]; then
echo "Failed test, return value: $rc" >&2
exit $rc
fi
}
-test_finish()
-{
+test_finish() {
set_orig
rm -f "${TEST_FILE}"
- if [ ! -z ${old_strict} ]; then
- echo ${old_strict} > ${WRITES_STRICT}
+ if [ -n "${old_strict}" ] && [ -w "${WRITES_STRICT}" ]; then
+ echo "${old_strict}" >"${WRITES_STRICT}"
fi
exit $rc
}
-run_numerictests()
-{
+run_numerictests() {
echo "== Testing sysctl behavior against ${TARGET} =="
rc=0
echo -n "Writing test file ... "
- echo "${TEST_STR}" > "${TEST_FILE}"
+ echo "${TEST_STR}" >"${TEST_FILE}"
if ! verify "${TEST_FILE}"; then
echo "FAIL" >&2
exit 1
@@ -296,7 +282,10 @@ run_numerictests()
echo -n "Writing sysctl with multiple long writes ... "
set_orig
- (perl -e 'print "A" x 50;'; echo "${TEST_STR}") | \
+ (
+ perl -e 'print "A" x 50;'
+ echo "${TEST_STR}"
+ ) |
dd of="${TARGET}" bs=50 2>/dev/null
if verify "${TARGET}"; then
echo "FAIL" >&2
@@ -307,13 +296,12 @@ run_numerictests()
test_rc
}
-check_failure()
-{
+check_failure() {
echo -n "Testing that $1 fails as expected..."
reset_vals
TEST_STR="$1"
orig="$(cat $TARGET)"
- echo -n "$TEST_STR" > $TARGET 2> /dev/null
+ echo -n "$TEST_STR" >$TARGET 2>/dev/null
# write should fail and $TARGET should retain its original value
if [ $? = 0 ] || [ "$(cat $TARGET)" != "$orig" ]; then
@@ -325,8 +313,7 @@ check_failure()
test_rc
}
-run_wideint_tests()
-{
+run_wideint_tests() {
# sysctl conversion functions receive a boolean sign and ulong
# magnitude; here we list the magnitudes we want to test (each of
# which will be tested in both positive and negative forms). Since
@@ -360,14 +347,16 @@ run_wideint_tests()
}
# Your test must accept digits 3 and 4 to use this
-run_limit_digit()
-{
+run_limit_digit() {
echo -n "Checking ignoring spaces up to PAGE_SIZE works on write ..."
reset_vals
- LIMIT=$((MAX_DIGITS -1))
+ LIMIT=$((MAX_DIGITS - 1))
TEST_STR="3"
- (perl -e 'print " " x '$LIMIT';'; echo "${TEST_STR}") | \
+ (
+ perl -e 'print " " x '$LIMIT';'
+ echo "${TEST_STR}"
+ ) |
dd of="${TARGET}" 2>/dev/null
if ! verify "${TARGET}"; then
@@ -383,7 +372,10 @@ run_limit_digit()
LIMIT=$((MAX_DIGITS))
TEST_STR="4"
- (perl -e 'print " " x '$LIMIT';'; echo "${TEST_STR}") | \
+ (
+ perl -e 'print " " x '$LIMIT';'
+ echo "${TEST_STR}"
+ ) |
dd of="${TARGET}" 2>/dev/null
if verify "${TARGET}"; then
@@ -396,12 +388,11 @@ run_limit_digit()
}
# You are using an int
-run_limit_digit_int()
-{
+run_limit_digit_int() {
echo -n "Testing INT_MAX works ..."
reset_vals
TEST_STR="$INT_MAX"
- echo -n $TEST_STR > $TARGET
+ echo -n $TEST_STR >$TARGET
if ! verify "${TARGET}"; then
echo "FAIL" >&2
@@ -414,7 +405,7 @@ run_limit_digit_int()
echo -n "Testing INT_MAX + 1 will fail as expected..."
reset_vals
let TEST_STR=$INT_MAX+1
- echo -n $TEST_STR > $TARGET 2> /dev/null
+ echo -n $TEST_STR >$TARGET 2>/dev/null
if verify "${TARGET}"; then
echo "FAIL" >&2
@@ -427,7 +418,7 @@ run_limit_digit_int()
echo -n "Testing negative values will work as expected..."
reset_vals
TEST_STR="-3"
- echo -n $TEST_STR > $TARGET 2> /dev/null
+ echo -n $TEST_STR >$TARGET 2>/dev/null
if ! verify "${TARGET}"; then
echo "FAIL" >&2
rc=1
@@ -438,11 +429,10 @@ run_limit_digit_int()
}
# You used an int array
-run_limit_digit_int_array()
-{
+run_limit_digit_int_array() {
echo -n "Testing array works as expected ... "
TEST_STR="4 3 2 1"
- echo -n $TEST_STR > $TARGET
+ echo -n $TEST_STR >$TARGET
if ! verify_diff_w "${TARGET}"; then
echo "FAIL" >&2
@@ -456,7 +446,7 @@ run_limit_digit_int_array()
# Do not reset_vals, carry on the values from the last test.
# If we only echo in two digits the last two are left intact
TEST_STR="100 101"
- echo -n $TEST_STR > $TARGET
+ echo -n $TEST_STR >$TARGET
# After we echo in, to help diff we need to set on TEST_STR what
# we expect the result to be.
TEST_STR="100 101 2 1"
@@ -473,9 +463,12 @@ run_limit_digit_int_array()
# Do not reset_vals, carry on the values from the last test.
# Even if you use an int array, you are still restricted to
# MAX_DIGITS, this is a known limitation. Test limit works.
- LIMIT=$((MAX_DIGITS -1))
+ LIMIT=$((MAX_DIGITS - 1))
TEST_STR="9"
- (perl -e 'print " " x '$LIMIT';'; echo "${TEST_STR}") | \
+ (
+ perl -e 'print " " x '$LIMIT';'
+ echo "${TEST_STR}"
+ ) |
dd of="${TARGET}" 2>/dev/null
TEST_STR="9 101 2 1"
@@ -492,7 +485,10 @@ run_limit_digit_int_array()
# Now go over limit.
LIMIT=$((MAX_DIGITS))
TEST_STR="7"
- (perl -e 'print " " x '$LIMIT';'; echo "${TEST_STR}") | \
+ (
+ perl -e 'print " " x '$LIMIT';'
+ echo "${TEST_STR}"
+ ) |
dd of="${TARGET}" 2>/dev/null
TEST_STR="7 101 2 1"
@@ -506,12 +502,11 @@ run_limit_digit_int_array()
}
# You are using an unsigned int
-run_limit_digit_uint()
-{
+run_limit_digit_uint() {
echo -n "Testing UINT_MAX works ..."
reset_vals
TEST_STR="$UINT_MAX"
- echo -n $TEST_STR > $TARGET
+ echo -n $TEST_STR >$TARGET
if ! verify "${TARGET}"; then
echo "FAIL" >&2
@@ -523,8 +518,8 @@ run_limit_digit_uint()
echo -n "Testing UINT_MAX + 1 will fail as expected..."
reset_vals
- TEST_STR=$(($UINT_MAX+1))
- echo -n $TEST_STR > $TARGET 2> /dev/null
+ TEST_STR=$(($UINT_MAX + 1))
+ echo -n $TEST_STR >$TARGET 2>/dev/null
if verify "${TARGET}"; then
echo "FAIL" >&2
@@ -537,7 +532,7 @@ run_limit_digit_uint()
echo -n "Testing negative values will not work as expected ..."
reset_vals
TEST_STR="-3"
- echo -n $TEST_STR > $TARGET 2> /dev/null
+ echo -n $TEST_STR >$TARGET 2>/dev/null
if verify "${TARGET}"; then
echo "FAIL" >&2
@@ -548,8 +543,7 @@ run_limit_digit_uint()
test_rc
}
-run_stringtests()
-{
+run_stringtests() {
echo -n "Writing entire sysctl in short writes ... "
set_orig
dd if="${TEST_FILE}" of="${TARGET}" bs=1 2>/dev/null
@@ -572,7 +566,7 @@ run_stringtests()
echo -n "Checking sysctl maxlen is at least $MAXLEN ... "
set_orig
- perl -e 'print "A" x ('"${MAXLEN}"'-2), "B";' | \
+ perl -e 'print "A" x ('"${MAXLEN}"'-2), "B";' |
dd of="${TARGET}" bs="${MAXLEN}" 2>/dev/null
if ! grep -q B "${TARGET}"; then
echo "FAIL" >&2
@@ -583,8 +577,8 @@ run_stringtests()
echo -n "Checking sysctl keeps original string on overflow append ... "
set_orig
- perl -e 'print "A" x ('"${MAXLEN}"'-1), "B";' | \
- dd of="${TARGET}" bs=$(( MAXLEN - 1 )) 2>/dev/null
+ perl -e 'print "A" x ('"${MAXLEN}"'-1), "B";' |
+ dd of="${TARGET}" bs=$((MAXLEN - 1)) 2>/dev/null
if grep -q B "${TARGET}"; then
echo "FAIL" >&2
rc=1
@@ -594,7 +588,7 @@ run_stringtests()
echo -n "Checking sysctl stays NULL terminated on write ... "
set_orig
- perl -e 'print "A" x ('"${MAXLEN}"'-1), "B";' | \
+ perl -e 'print "A" x ('"${MAXLEN}"'-1), "B";' |
dd of="${TARGET}" bs="${MAXLEN}" 2>/dev/null
if grep -q B "${TARGET}"; then
echo "FAIL" >&2
@@ -605,8 +599,8 @@ run_stringtests()
echo -n "Checking sysctl stays NULL terminated on overwrite ... "
set_orig
- perl -e 'print "A" x ('"${MAXLEN}"'-1), "BB";' | \
- dd of="${TARGET}" bs=$(( $MAXLEN + 1 )) 2>/dev/null
+ perl -e 'print "A" x ('"${MAXLEN}"'-1), "BB";' |
+ dd of="${TARGET}" bs=$(($MAXLEN + 1)) 2>/dev/null
if grep -q B "${TARGET}"; then
echo "FAIL" >&2
rc=1
@@ -617,12 +611,11 @@ run_stringtests()
test_rc
}
-target_exists()
-{
+target_exists() {
TARGET="${SYSCTL}/$1"
TEST_ID="$2"
- if [ ! -f ${TARGET} ] ; then
+ if [ ! -f ${TARGET} ]; then
echo "Target for test $TEST_ID: $TARGET not exist, skipping test ..."
return 0
fi
@@ -659,9 +652,9 @@ run_bitmaptest() {
echo -n "Checking bitmap handler... "
TEST_FILE=$(mktemp)
- echo -n "$TEST_STR" > $TEST_FILE
+ echo -n "$TEST_STR" >$TEST_FILE
- cat $TEST_FILE > $TARGET 2> /dev/null
+ cat $TEST_FILE >$TARGET 2>/dev/null
if [ $? -ne 0 ]; then
echo "FAIL" >&2
rc=1
@@ -678,20 +671,18 @@ run_bitmaptest() {
test_rc
}
-sysctl_test_0001()
-{
+sysctl_test_0001() {
TARGET="${SYSCTL}/$(get_test_target 0001)"
reset_vals
ORIG=$(cat "${TARGET}")
- TEST_STR=$(( $ORIG + 1 ))
+ TEST_STR=$(($ORIG + 1))
run_numerictests
run_wideint_tests
run_limit_digit
}
-sysctl_test_0002()
-{
+sysctl_test_0002() {
TARGET="${SYSCTL}/$(get_test_target 0002)"
reset_vals
ORIG=$(cat "${TARGET}")
@@ -703,12 +694,11 @@ sysctl_test_0002()
run_stringtests
}
-sysctl_test_0003()
-{
+sysctl_test_0003() {
TARGET="${SYSCTL}/$(get_test_target 0003)"
reset_vals
ORIG=$(cat "${TARGET}")
- TEST_STR=$(( $ORIG + 1 ))
+ TEST_STR=$(($ORIG + 1))
run_numerictests
run_wideint_tests
@@ -716,12 +706,11 @@ sysctl_test_0003()
run_limit_digit_int
}
-sysctl_test_0004()
-{
+sysctl_test_0004() {
TARGET="${SYSCTL}/$(get_test_target 0004)"
reset_vals
ORIG=$(cat "${TARGET}")
- TEST_STR=$(( $ORIG + 1 ))
+ TEST_STR=$(($ORIG + 1))
run_numerictests
run_wideint_tests
@@ -729,8 +718,7 @@ sysctl_test_0004()
run_limit_digit_uint
}
-sysctl_test_0005()
-{
+sysctl_test_0005() {
TARGET="${SYSCTL}/$(get_test_target 0005)"
reset_vals
ORIG=$(cat "${TARGET}")
@@ -738,16 +726,14 @@ sysctl_test_0005()
run_limit_digit_int_array
}
-sysctl_test_0006()
-{
+sysctl_test_0006() {
TARGET="${SYSCTL}/bitmap_0001"
reset_vals
ORIG=""
run_bitmaptest
}
-sysctl_test_0007()
-{
+sysctl_test_0007() {
TARGET="${SYSCTL}/boot_int"
if [ ! -f $TARGET ]; then
echo "Skipping test for $TARGET as it is not present ..."
@@ -786,8 +772,7 @@ sysctl_test_0007()
return $ksft_skip
}
-sysctl_test_0008()
-{
+sysctl_test_0008() {
TARGET="${SYSCTL}/match_int"
if [ ! -f $TARGET ]; then
echo "Skipping test for $TARGET as it is not present ..."
@@ -807,8 +792,7 @@ sysctl_test_0008()
return 0
}
-list_tests()
-{
+list_tests() {
echo "Test ID list:"
echo
echo "TEST_ID x NUM_TEST"
@@ -825,8 +809,7 @@ list_tests()
echo "0008 x $(get_test_count 0008) - tests sysctl macro values match"
}
-usage()
-{
+usage() {
NUM_TESTS=$(grep -o ' ' <<<"$ALL_TESTS" | grep -c .)
let NUM_TESTS=$NUM_TESTS+1
MAX_TEST=$(printf "%04d\n" $NUM_TESTS)
@@ -860,38 +843,33 @@ usage()
exit 1
}
-function test_num()
-{
+function test_num() {
re='^[0-9]+$'
if ! [[ $1 =~ $re ]]; then
usage
fi
}
-function get_test_count()
-{
+function get_test_count() {
test_num $1
TEST_DATA=$(echo $ALL_TESTS | awk '{print $'$1'}')
echo ${TEST_DATA} | awk -F":" '{print $2}'
}
-function get_test_enabled()
-{
+function get_test_enabled() {
test_num $1
TEST_DATA=$(echo $ALL_TESTS | awk '{print $'$1'}')
echo ${TEST_DATA} | awk -F":" '{print $3}'
}
-function get_test_target()
-{
+function get_test_target() {
test_num $1
TEST_DATA=$(echo $ALL_TESTS | awk '{print $'$1'}')
echo ${TEST_DATA} | awk -F":" '{print $4}'
}
-function run_all_tests()
-{
- for i in $ALL_TESTS ; do
+function run_all_tests() {
+ for i in $ALL_TESTS; do
TEST_ID=${i%:*:*:*}
ENABLED=$(get_test_enabled $TEST_ID)
TEST_COUNT=$(get_test_count $TEST_ID)
@@ -905,8 +883,7 @@ function run_all_tests()
done
}
-function watch_log()
-{
+function watch_log() {
if [ $# -ne 3 ]; then
clear
fi
@@ -914,8 +891,7 @@ function watch_log()
echo "Running test: $2 - run #$1"
}
-function watch_case()
-{
+function watch_case() {
i=0
while [ 1 ]; do
@@ -931,8 +907,7 @@ function watch_case()
done
}
-function test_case()
-{
+function test_case() {
NUM_TESTS=$2
i=0
@@ -950,8 +925,7 @@ function test_case()
done
}
-function parse_args()
-{
+function parse_args() {
if [ $# -eq 0 ]; then
run_all_tests
else
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] fix(script): improve safety and formatting for WRITES_STRICT restore
2025-04-23 8:32 [PATCH] fix(script): improve safety and formatting for WRITES_STRICT restore Alexander Shatalin
@ 2025-04-28 20:52 ` Shuah Khan
0 siblings, 0 replies; 2+ messages in thread
From: Shuah Khan @ 2025-04-28 20:52 UTC (permalink / raw)
To: Alexander Shatalin, linux-kselftest
On 4/23/25 02:32, Alexander Shatalin wrote:
> From: Alexander Shatalin <sashatalin03@gmail.com <mailto:sashatalin03@gmail.com>>
>
> This patch improves the safety when restoring WRITES_STRICT by ensuring:
> - The variable is not empty (`-n`)
> - The target file is writable (`-w`)
> Also improves formatting by quoting variables and following shell best practices.
>
> Signed-off-by: Alexander Shatalin <sashatalin03@gmail.com <mailto:sashatalin03@gmail.com>>
No attachments please. Send a proper patch with correct short log
and change log.
Check submitting patches documentation for details.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-04-28 20:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-23 8:32 [PATCH] fix(script): improve safety and formatting for WRITES_STRICT restore Alexander Shatalin
2025-04-28 20:52 ` Shuah Khan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox