[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <202402070452.24B3200@keescook>
Date: Wed, 7 Feb 2024 04:53:35 -0800
From: Kees Cook <keescook@...omium.org>
To: Yuanhe Shu <xiangzao@...ux.alibaba.com>
Cc: tony.luck@...el.com, gpiccoli@...lia.com, shuah@...nel.org,
corbet@....net, xlpang@...ux.alibaba.com,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
linux-hardening@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH 3/3] tools/testing: adjust pstore backend related selftest
On Wed, Feb 07, 2024 at 10:19:21AM +0800, Yuanhe Shu wrote:
> Pstore now supports multiple backends, the module parameter
> pstore.backend varies from 'registered backend' to 'backends that are
> allowed to register'. Adjust selftests to match the change.
>
> Signed-off-by: Yuanhe Shu <xiangzao@...ux.alibaba.com>
> ---
> tools/testing/selftests/pstore/common_tests | 8 +--
> .../selftests/pstore/pstore_post_reboot_tests | 65 ++++++++++---------
> tools/testing/selftests/pstore/pstore_tests | 2 +-
> 3 files changed, 38 insertions(+), 37 deletions(-)
>
> diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
> index 4509f0cc9c91..497e6fc3215f 100755
> --- a/tools/testing/selftests/pstore/common_tests
> +++ b/tools/testing/selftests/pstore/common_tests
> @@ -27,9 +27,9 @@ show_result() { # result_value
> }
>
> check_files_exist() { # type of pstorefs file
> - if [ -e ${1}-${backend}-0 ]; then
> + if [ -e ${1}-${2}-0 ]; then
> prlog "ok"
> - for f in `ls ${1}-${backend}-*`; do
> + for f in `ls ${1}-${2}-*`; do
> prlog -e "\t${f}"
> done
> else
> @@ -74,9 +74,9 @@ prlog "=== Pstore unit tests (`basename $0`) ==="
> prlog "UUID="$UUID
>
> prlog -n "Checking pstore backend is registered ... "
> -backend=`cat /sys/module/pstore/parameters/backend`
> +backends=$(dmesg | sed -n 's/.*pstore: Registered \(.*\) as persistent store backend.*/\1/p')
> show_result $?
> -prlog -e "\tbackend=${backend}"
> +prlog -e "\tbackends="$backends
Missing trailing "? Also, doesn't this end up printing multiple lines?
Perhaps, like LSM stacking, we need a /sys/module entry for the list of
backends, comma separated?
> prlog -e "\tcmdline=`cat /proc/cmdline`"
> if [ $rc -ne 0 ]; then
> exit 1
> diff --git a/tools/testing/selftests/pstore/pstore_post_reboot_tests b/tools/testing/selftests/pstore/pstore_post_reboot_tests
> index d6da5e86efbf..9e40ccb9c918 100755
> --- a/tools/testing/selftests/pstore/pstore_post_reboot_tests
> +++ b/tools/testing/selftests/pstore/pstore_post_reboot_tests
> @@ -36,45 +36,46 @@ else
> fi
>
> cd ${mount_point}
> +for backend in ${backends}; do
> + prlog -n "Checking ${backend}-dmesg files exist in pstore filesystem ... "
> + check_files_exist dmesg ${backend}
>
> -prlog -n "Checking dmesg files exist in pstore filesystem ... "
> -check_files_exist dmesg
> + prlog -n "Checking ${backend}-console files exist in pstore filesystem ... "
> + check_files_exist console ${backend}
>
> -prlog -n "Checking console files exist in pstore filesystem ... "
> -check_files_exist console
> + prlog -n "Checking ${backend}-pmsg files exist in pstore filesystem ... "
> + check_files_exist pmsg ${backend}
>
> -prlog -n "Checking pmsg files exist in pstore filesystem ... "
> -check_files_exist pmsg
> + prlog -n "Checking ${backend}-dmesg files contain oops end marker"
> + grep_end_trace() {
> + grep -q "\---\[ end trace" $1
> + }
> + files=`ls dmesg-${backend}-*`
> + operate_files $? "$files" grep_end_trace
>
> -prlog -n "Checking dmesg files contain oops end marker"
> -grep_end_trace() {
> - grep -q "\---\[ end trace" $1
> -}
> -files=`ls dmesg-${backend}-*`
> -operate_files $? "$files" grep_end_trace
> + prlog -n "Checking ${backend}-console file contains oops end marker ... "
> + grep -q "\---\[ end trace" console-${backend}-0
> + show_result $?
>
> -prlog -n "Checking console file contains oops end marker ... "
> -grep -q "\---\[ end trace" console-${backend}-0
> -show_result $?
> -
> -prlog -n "Checking pmsg file properly keeps the content written before crash ... "
> -prev_uuid=`cat $TOP_DIR/prev_uuid`
> -if [ $? -eq 0 ]; then
> - nr_matched=`grep -c "$TEST_STRING_PATTERN" pmsg-${backend}-0`
> - if [ $nr_matched -eq 1 ]; then
> - grep -q "$TEST_STRING_PATTERN"$prev_uuid pmsg-${backend}-0
> - show_result $?
> + prlog -n "Checking ${backend}-pmsg file properly keeps the content written before crash ... "
> + prev_uuid=`cat $TOP_DIR/prev_uuid`
> + if [ $? -eq 0 ]; then
> + nr_matched=`grep -c "$TEST_STRING_PATTERN" pmsg-${backend}-0`
> + if [ $nr_matched -eq 1 ]; then
> + grep -q "$TEST_STRING_PATTERN"$prev_uuid pmsg-${backend}-0
> + show_result $?
> + else
> + prlog "FAIL"
> + rc=1
> + fi
> else
> - prlog "FAIL"
> - rc=1
> + prlog "FAIL"
> + rc=1
> fi
> -else
> - prlog "FAIL"
> - rc=1
> -fi
>
> -prlog -n "Removing all files in pstore filesystem "
> -files=`ls *-${backend}-*`
> -operate_files $? "$files" rm
> + prlog -n "Removing all ${backend} files in pstore filesystem "
> + files=`ls *-${backend}-*`
> + operate_files $? "$files" rm
> +done
>
> exit $rc
> diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
> index 2aa9a3852a84..f4665a8c77dc 100755
> --- a/tools/testing/selftests/pstore/pstore_tests
> +++ b/tools/testing/selftests/pstore/pstore_tests
> @@ -10,7 +10,7 @@
> . ./common_tests
>
> prlog -n "Checking pstore console is registered ... "
> -dmesg | grep -Eq "console \[(pstore|${backend})"
> +dmesg | grep -Eq "console \[(pstore console)"
> show_result $?
>
> prlog -n "Checking /dev/pmsg0 exists ... "
> --
> 2.39.3
>
Otherwise seems ok
--
Kees Cook
Powered by blists - more mailing lists