Re: [PATCH v13 07/12] selftests: tracing: Add a basic testcase for wprobe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unreachable error handling logic due to early exit under 'set -e'.
--

--- Patch [7]: [PATCH v13 07/12] selftests: tracing: Add a basic testcase for wprobe ---
commit 6d72a05750c5dfc08de6b03a6fc65e5144b20b9f
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

selftests: tracing: Add a basic testcase for wprobe

Add an add_remove_wprobe.tc testcase for testing wprobe events that
tests adding and removing operations.

> diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc
> @@ -0,0 +1,63 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +# description: Generic dynamic event - add/remove wprobe events
> +# requires: dynamic_events "w[:[<group>/][<event>]] [r|w|rw]@<addr>[:<len>]":README
> +
> +echo 0 > events/enable
> +echo > dynamic_events
> +
> +# Use jiffies as a variable that is frequently written to.
> +TARGET=jiffies
> +
> +echo "w:my_wprobe w@$TARGET" >> dynamic_events
> +
> +if ! grep -q my_wprobe dynamic_events; then
> +    echo "Failed to create wprobe event"
> +    exit_fail
> +fi

[Severity: Medium]
Since the ftracetest framework runs these scripts with set -e (errexit) 
enabled, will this script exit immediately if the kernel rejects the event 
creation and the echo command fails?

If so, the subsequent diagnostic grep check and exit_fail call wouldn't be
reached, leading to a silent failure without the intended error message.

> +
> +if [ ! -d events/wprobes/my_wprobe ]; then
> +    echo "Failed to create wprobe event directory"
> +    exit_fail
> +fi
> +
> +echo 1 > events/wprobes/my_wprobe/enable
> +
> +# Check if the event is enabled
> +if ! grep -q 1 events/wprobes/my_wprobe/enable; then
> +    echo "Failed to enable wprobe event"
> +    exit_fail
> +fi

[Severity: Medium]
Similarly, if enabling the wprobe fails at the sysfs boundary, would this
echo cause an early exit and bypass the diagnostic error message here as well?

[ ... ]

> +echo "-:my_wprobe" >> dynamic_events
> +
> +if grep -q my_wprobe dynamic_events; then
> +    echo "Failed to remove wprobe event"
> +    exit_fail
> +fi

[Severity: Medium]
Could this echo also trigger an early exit if removing the wprobe fails,
bypassing the failure message?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178739053919.1520941.17662338993878200834.stgit@devnote2?part=7