[PATCH] fault-injection: Enhance failcmd to exit on non-hex address input

[PATCH] fault-injection: Enhance failcmd to exit on non-hex address input

[Breno Leitao]

The failcmd.sh script in the fault-injection toolkit does not currently
validate whether the provided address is in hexadecimal format. This can
lead to silent failures if the address is sourced from places like
`/proc/kallsyms`, which omits the '0x' prefix, potentially causing users
to operate under incorrect assumptions.

Introduce a new function, `exit_if_not_hex`, which checks the format of
the provided address and exits with an error message if the address is
not a valid hexadecimal number.

This enhancement prevents users from running the command with
improperly formatted addresses, thus improving the robustness and
usability of the failcmd tool.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 tools/testing/fault-injection/failcmd.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 mode change 100644 => 100755 tools/testing/fault-injection/failcmd.sh

diff --git a/tools/testing/fault-injection/failcmd.sh b/tools/testing/fault-injection/failcmd.sh
old mode 100644
new mode 100755
index 78dac34264be..234d49fc49d9
--- a/tools/testing/fault-injection/failcmd.sh
+++ b/tools/testing/fault-injection/failcmd.sh
@@ -64,6 +64,14 @@ ENVIRONMENT
 EOF
 }
 
+exit_if_not_hex() {
+    local value="$1"
+    if ! [[ $value =~ ^0x[0-9a-fA-F]+$ ]]; then
+        echo "Error: The provided value '$value' is not a valid hexadecimal number."
+        exit 1
+    fi
+}
+
 if [ $UID != 0 ]; then
 	echo must be run as root >&2
 	exit 1
@@ -160,18 +168,22 @@ while true; do
 		shift 2
 		;;
 	--require-start)
+		exit_if_not_hex "$2"
 		echo $2 > $FAULTATTR/require-start
 		shift 2
 		;;
 	--require-end)
+		exit_if_not_hex "$2"
 		echo $2 > $FAULTATTR/require-end
 		shift 2
 		;;
 	--reject-start)
+		exit_if_not_hex "$2"
 		echo $2 > $FAULTATTR/reject-start
 		shift 2
 		;;
 	--reject-end)
+		exit_if_not_hex "$2"
 		echo $2 > $FAULTATTR/reject-end
 		shift 2
 		;;
-- 
2.43.0

Re: [PATCH] fault-injection: Enhance failcmd to exit on non-hex address input

[Akinobu Mita]

2024年7月26日(金) 19:50 Breno Leitao <leitao@debian.org>:
>
> The failcmd.sh script in the fault-injection toolkit does not currently
> validate whether the provided address is in hexadecimal format. This can
> lead to silent failures if the address is sourced from places like
> `/proc/kallsyms`, which omits the '0x' prefix, potentially causing users
> to operate under incorrect assumptions.
>
> Introduce a new function, `exit_if_not_hex`, which checks the format of
> the provided address and exits with an error message if the address is
> not a valid hexadecimal number.
>
> This enhancement prevents users from running the command with
> improperly formatted addresses, thus improving the robustness and
> usability of the failcmd tool.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  tools/testing/fault-injection/failcmd.sh | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>  mode change 100644 => 100755 tools/testing/fault-injection/failcmd.sh
>
> diff --git a/tools/testing/fault-injection/failcmd.sh b/tools/testing/fault-injection/failcmd.sh
> old mode 100644
> new mode 100755
> index 78dac34264be..234d49fc49d9
> --- a/tools/testing/fault-injection/failcmd.sh
> +++ b/tools/testing/fault-injection/failcmd.sh
> @@ -64,6 +64,14 @@ ENVIRONMENT
>  EOF
>  }
>
> +exit_if_not_hex() {
> +    local value="$1"
> +    if ! [[ $value =~ ^0x[0-9a-fA-F]+$ ]]; then
> +        echo "Error: The provided value '$value' is not a valid hexadecimal number."

It is better to write error messages to standard error rather than
standard output.

Other than that I think it's good.

> +        exit 1
> +    fi
> +}
> +
>  if [ $UID != 0 ]; then
>         echo must be run as root >&2
>         exit 1
> @@ -160,18 +168,22 @@ while true; do
>                 shift 2
>                 ;;
>         --require-start)
> +               exit_if_not_hex "$2"
>                 echo $2 > $FAULTATTR/require-start
>                 shift 2
>                 ;;
>         --require-end)
> +               exit_if_not_hex "$2"
>                 echo $2 > $FAULTATTR/require-end
>                 shift 2
>                 ;;
>         --reject-start)
> +               exit_if_not_hex "$2"
>                 echo $2 > $FAULTATTR/reject-start
>                 shift 2
>                 ;;
>         --reject-end)
> +               exit_if_not_hex "$2"
>                 echo $2 > $FAULTATTR/reject-end
>                 shift 2
>                 ;;
> --
> 2.43.0
>

Re: [PATCH] fault-injection: Enhance failcmd to exit on non-hex address input

[Breno Leitao]

Hello Akinobu,

On Sun, Jul 28, 2024 at 06:00:14PM +0900, Akinobu Mita wrote:
> 2024年7月26日(金) 19:50 Breno Leitao <leitao@debian.org>:

> > +exit_if_not_hex() {
> > +    local value="$1"
> > +    if ! [[ $value =~ ^0x[0-9a-fA-F]+$ ]]; then
> > +        echo "Error: The provided value '$value' is not a valid hexadecimal number."
> 
> It is better to write error messages to standard error rather than
> standard output.

Agree. I've sent a V2 with your proposed change. Thanks!

> Other than that I think it's good.

Thanks.

By the way, this file seems unmaintained by looking at MAINTAINERS.
Would you mind if I send something as:


Author: Breno Leitao <leitao@debian.org>
Date:   Mon Jul 29 01:53:44 2024 -0700

    failcmd: Add script file in MAINTAINERS
    
    failcmd is one of the main interfaces to fault injection framework, but,
    it is not listed under FAULT INJECTION SUPPORT entry in MAINTAINERS.
    This is unfortunate, since git-send-email doesn't find emails to send
    the patches to, forcing the user to try to guess who maintains it.
    
    Akinobu Mita seems to be actively maintaining it, so, let's add the file
    under FAULT INJECTION SUPPORT section.
    
    Signed-off-by: Breno Leitao <leitao@debian.org>

diff --git a/MAINTAINERS b/MAINTAINERS
index 0748d6bd0c4f..11f3ef0b5ffd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8388,6 +8388,7 @@ M:	Akinobu Mita <akinobu.mita@gmail.com>
 S:	Supported
 F:	Documentation/fault-injection/
 F:	lib/fault-inject.c
+F:	tools/testing/fault-injection/
 
 FBTFT Framebuffer drivers
 L:	dri-devel@lists.freedesktop.org

Re: [PATCH] fault-injection: Enhance failcmd to exit on non-hex address input

[Akinobu Mita]

2024年7月29日(月) 17:59 Breno Leitao <leitao@debian.org>:
>
> Hello Akinobu,
>
> On Sun, Jul 28, 2024 at 06:00:14PM +0900, Akinobu Mita wrote:
> > 2024年7月26日(金) 19:50 Breno Leitao <leitao@debian.org>:
>
> > > +exit_if_not_hex() {
> > > +    local value="$1"
> > > +    if ! [[ $value =~ ^0x[0-9a-fA-F]+$ ]]; then
> > > +        echo "Error: The provided value '$value' is not a valid hexadecimal number."
> >
> > It is better to write error messages to standard error rather than
> > standard output.
>
> Agree. I've sent a V2 with your proposed change. Thanks!
>
> > Other than that I think it's good.
>
> Thanks.
>
> By the way, this file seems unmaintained by looking at MAINTAINERS.
> Would you mind if I send something as:

No problem.

> Author: Breno Leitao <leitao@debian.org>
> Date:   Mon Jul 29 01:53:44 2024 -0700
>
>     failcmd: Add script file in MAINTAINERS
>
>     failcmd is one of the main interfaces to fault injection framework, but,
>     it is not listed under FAULT INJECTION SUPPORT entry in MAINTAINERS.
>     This is unfortunate, since git-send-email doesn't find emails to send
>     the patches to, forcing the user to try to guess who maintains it.
>
>     Akinobu Mita seems to be actively maintaining it, so, let's add the file
>     under FAULT INJECTION SUPPORT section.
>
>     Signed-off-by: Breno Leitao <leitao@debian.org>

Reviewed-by: Akinobu Mita <akinobu.mita@gmail.com>

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0748d6bd0c4f..11f3ef0b5ffd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8388,6 +8388,7 @@ M:        Akinobu Mita <akinobu.mita@gmail.com>
>  S:     Supported
>  F:     Documentation/fault-injection/
>  F:     lib/fault-inject.c
> +F:     tools/testing/fault-injection/
>
>  FBTFT Framebuffer drivers
>  L:     dri-devel@lists.freedesktop.org