Re: [PATCH bpf-next v3 3/3] selftests/bpf: Test modified syscall ctx for ARG_PTR_TO_CTX

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:

> Ensure that global subprogs and tail calls can only accept an unmodified
> PTR_TO_CTX for syscall programs. For all other program types, fixed or
> variable offsets on PTR_TO_CTX is rejected when passed into an argument
> of any call instruction type, through the unified logic of
> check_func_arg_reg_off.
>
> Finally, add a positive example of a case that should succeed with all
> our previous changes.
>
> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  .../bpf/progs/verifier_global_subprogs.c      | 94 +++++++++++++++++++
>  1 file changed, 94 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> index 2250fc31574d..1e08aff7532e 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> @@ -357,6 +357,100 @@ int arg_tag_ctx_syscall(void *ctx)
>  	return tracing_subprog_void(ctx) + tracing_subprog_u64(ctx) + tp_whatever(ctx);
>  }
>  
> +__weak int syscall_array_bpf_for(void *ctx __arg_ctx)
> +{
> +	int *arr = ctx;
> +	int i;
> +
> +	bpf_for(i, 0, 100)
> +		arr[i] *= i;
> +
> +	return 0;
> +}
> +
> +SEC("?syscall")
> +__success __log_level(2)
> +int arg_tag_ctx_syscall_bpf_for(void *ctx)
> +{
> +	return syscall_array_bpf_for(ctx);
> +}
> +
> +SEC("syscall")
> +__auxiliary
> +int syscall_tailcall_target(void *ctx)
> +{
> +	return syscall_array_bpf_for(ctx);
> +}
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
> +	__uint(max_entries, 1);
> +	__uint(key_size, sizeof(__u32));
> +	__array(values, int (void *));
> +} syscall_prog_array SEC(".maps") = {
> +	.values = {
> +		[0] = (void *)&syscall_tailcall_target,
> +	},
> +};
> +
> +SEC("?syscall")
> +__success __log_level(2)
> +int arg_tag_ctx_syscall_tailcall(void *ctx)
> +{
> +	bpf_tail_call(ctx, &syscall_prog_array, 0);
> +	return 0;
> +}
> +
> +SEC("?syscall")
> +__failure __log_level(2)
> +__msg("dereference of modified ctx ptr R1 off=8 disallowed")
> +int arg_tag_ctx_syscall_tailcall_fixed_off_bad(void *ctx)
> +{
> +	char *p = ctx;
> +
> +	p += 8;
> +	bpf_tail_call(p, &syscall_prog_array, 0);
> +	return 0;
> +}
> +
> +SEC("?syscall")
> +__failure __log_level(2)
> +__msg("variable ctx access var_off=(0x0; 0x4) disallowed")
> +int arg_tag_ctx_syscall_tailcall_var_off_bad(void *ctx)
> +{
> +	__u64 off = bpf_get_prandom_u32();
> +	char *p = ctx;
> +
> +	off &= 4;
> +	p += off;
> +	bpf_tail_call(p, &syscall_prog_array, 0);
> +	return 0;
> +}
> +
> +SEC("?syscall")
> +__failure __log_level(2)
> +__msg("dereference of modified ctx ptr R1 off=8 disallowed")
> +int arg_tag_ctx_syscall_fixed_off_bad(void *ctx)
> +{
> +	char *p = ctx;
> +
> +	p += 8;
> +	return subprog_ctx_tag(p);
> +}
> +
> +SEC("?syscall")
> +__failure __log_level(2)
> +__msg("variable ctx access var_off=(0x0; 0x4) disallowed")
> +int arg_tag_ctx_syscall_var_off_bad(void *ctx)
Test cases for subprog_ctx_tag() and bpf_tail_call() are duplicated,
even verifier error messages are the same. I don't see a better way to
avoid it though (macros as in prev patch look worse).
Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
> +{
> +	__u64 off = bpf_get_prandom_u32();
> +	char *p = ctx;
> +
> +	off &= 4;
> +	p += off;
> +	return subprog_ctx_tag(p);
> +}
> +
>  __weak int subprog_dynptr(struct bpf_dynptr *dptr)
>  {
>  	long *d, t, buf[1] = {};
> -- 
> 2.52.0