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

[Yonghong Song <yonghong.song@linux.dev>]

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 <yonghong.song@linux.dev>
> ---
>   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";