[bpf,2/2] selftests/bpf: Add a test to verify previous stacksafe() fix

Commit Message

Yonghong Song Aug. 12, 2024, 5:21 a.m. UTC

A selftest is added such that without the previous patch,
a crash can happen. With the previous patch, the test can
run successfully. The new test is written in a way which
mimics original crash case:
  main_prog
    static_prog_1
      static_prog_2
where static_prog_1 has different paths to static_prog_2
and some path has stack allocated and some other path
does not. A stacksafe() checking in static_prog_2()
triggered the crash.

Signed-off-by: Yonghong Song <[e-mail]>
---
 tools/testing/selftests/bpf/progs/iters.c | 54 +++++++++++++++++++++++
 1 file changed, 54 insertions(+)

Comments

Yonghong Song Aug. 12, 2024, 3:07 p.m. UTC | #1

On 8/11/24 10:21 PM, Yonghong Song wrote:
> A selftest is added such that without the previous patch,
> a crash can happen. With the previous patch, the test can
> run successfully. The new test is written in a way which
> mimics original crash case:
>    main_prog
>      static_prog_1
>        static_prog_2
> where static_prog_1 has different paths to static_prog_2
> and some path has stack allocated and some other path
> does not. A stacksafe() checking in static_prog_2()
> triggered the crash.
>
> Signed-off-by: Yonghong Song <[e-mail]>
> ---
>   tools/testing/selftests/bpf/progs/iters.c | 54 +++++++++++++++++++++++
>   1 file changed, 54 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/iters.c b/tools/testing/selftests/bpf/progs/iters.c
> index 16bdc3e25591..8d3b75147617 100644
> --- a/tools/testing/selftests/bpf/progs/iters.c
> +++ b/tools/testing/selftests/bpf/progs/iters.c
> @@ -1432,4 +1432,58 @@ int iter_arr_with_actual_elem_count(const void *ctx)
>   	return sum;
>   }
>   
> +__u32 upper, select_n, result;
> +__u64 global;
> +
> +static __noinline bool nest_2(char *str, int len)

Argument 'len' is not needed here. I can make the change after some
additional comments.

> +{
> +	/* some insns (including branch insns) to ensure stacksafe() is triggered
> +	 * in nest_2(). This way, stacksafe() can compare frame associated with nest_1().
> +	 */
> +	if (str[0] == 't')
> +		return true;
> +	if (str[1] == 'e')
> +		return true;
> +	if (str[2] == 's')
> +		return true;
> +	if (str[3] == 't')
> +		return true;
> +	return false;
> +}
> +
> +static __noinline bool nest_1(int n)
> +{
> +	/* case 0: allocate stack, case 1: no allocate stack */
> +	switch (n) {
> +	case 0: {
> +		char comm[16];
> +
> +		if (bpf_get_current_comm(comm, 16))
> +			return false;
> +		return nest_2(comm, 16);
> +	}
> +	case 1:
> +		return nest_2((char *)&global, sizeof(global));
> +	default:
> +		return false;
> +	}

To triger the failure, we rely on 'case 0' is explored by the verifier first
and 'case 1' is explored later. This seems the case for llvm18 and llvm20.

> +}
> +
> +SEC("raw_tp")
> +__success
> +int iter_subprog_check_stacksafe(const void *ctx)
> +{
> +	long i;
> +
> +	bpf_for(i, 0, upper) {
> +		if (!nest_1(select_n)) {
> +			result = 1;
> +			return 0;
> +		}
> +	}
> +
> +	result = 2;
> +	return 0;
> +}
> +
>   char _license[] SEC("license") = "GPL";

Patch

diff --git a/tools/testing/selftests/bpf/progs/iters.c b/tools/testing/selftests/bpf/progs/iters.c
index 16bdc3e25591..8d3b75147617 100644
--- a/tools/testing/selftests/bpf/progs/iters.c
+++ b/tools/testing/selftests/bpf/progs/iters.c
@@ -1432,4 +1432,58 @@  int iter_arr_with_actual_elem_count(const void *ctx)
 	return sum;
 }
 
+__u32 upper, select_n, result;
+__u64 global;
+
+static __noinline bool nest_2(char *str, int len)
+{
+	/* some insns (including branch insns) to ensure stacksafe() is triggered
+	 * in nest_2(). This way, stacksafe() can compare frame associated with nest_1().
+	 */
+	if (str[0] == 't')
+		return true;
+	if (str[1] == 'e')
+		return true;
+	if (str[2] == 's')
+		return true;
+	if (str[3] == 't')
+		return true;
+	return false;
+}
+
+static __noinline bool nest_1(int n)
+{
+	/* case 0: allocate stack, case 1: no allocate stack */
+	switch (n) {
+	case 0: {
+		char comm[16];
+
+		if (bpf_get_current_comm(comm, 16))
+			return false;
+		return nest_2(comm, 16);
+	}
+	case 1:
+		return nest_2((char *)&global, sizeof(global));
+	default:
+		return false;
+	}
+}
+
+SEC("raw_tp")
+__success
+int iter_subprog_check_stacksafe(const void *ctx)
+{
+	long i;
+
+	bpf_for(i, 0, upper) {
+		if (!nest_1(select_n)) {
+			result = 1;
+			return 0;
+		}
+	}
+
+	result = 2;
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";