netdev - Re: [PATCH bpf-next v6 06/23] selftests/bpf: Add tests for kfunc returning a memory pointer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 15 Jul 2022 21:33:32 -0700
From:   Yonghong Song <yhs@...com>
To:     Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Jiri Kosina <jikos@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>, Shuah Khan <shuah@...nel.org>,
        Dave Marchevsky <davemarchevsky@...com>,
        Joe Stringer <joe@...ium.io>, Jonathan Corbet <corbet@....net>
Cc:     Tero Kristo <tero.kristo@...ux.intel.com>,
        linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
        netdev@...r.kernel.org, bpf@...r.kernel.org,
        linux-kselftest@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH bpf-next v6 06/23] selftests/bpf: Add tests for kfunc
 returning a memory pointer



On 7/12/22 7:58 AM, Benjamin Tissoires wrote:
> We add 2 new kfuncs that are following the RET_PTR_TO_MEM
> capability from the previous commit.
> Then we test them in selftests:
> the first tests are testing valid case, and are not failing,
> and the later ones are actually preventing the program to be loaded
> because they are wrong.
> 
> To work around that, we mark the failing ones as not autoloaded
> (with SEC("?tc")), and we manually enable them one by one, ensuring
> the verifier rejects them.
> 
> To be able to use bpf_program__set_autoload() from libbpf, we need
> to use a plain skeleton, not a light-skeleton, and this is why we
> also change the Makefile to generate both for kfunc_call_test.c
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
> 
> ---
> 
> new in v6
> ---
>   include/linux/btf.h                           |  4 +-
>   net/bpf/test_run.c                            | 22 +++++
>   tools/testing/selftests/bpf/Makefile          |  5 +-
>   .../selftests/bpf/prog_tests/kfunc_call.c     | 48 ++++++++++
>   .../selftests/bpf/progs/kfunc_call_test.c     | 89 +++++++++++++++++++
>   5 files changed, 165 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 31da4273c2ec..6f46ff2128ae 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -422,7 +422,9 @@ static inline int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dt
>   
>   static inline bool btf_type_is_struct_ptr(struct btf *btf, const struct btf_type *t)
>   {
> -	/* t comes in already as a pointer */
> +	if (!btf_type_is_ptr(t))
> +		return false;

Why we have a change here?

> +
>   	t = btf_type_by_id(btf, t->type);
>   
>   	/* allow const */
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 9da2a42811e8..0b4026ea4652 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -606,6 +606,24 @@ noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p)
>   	WARN_ON_ONCE(1);
>   }
>   
> +static int *__bpf_kfunc_call_test_get_mem(struct prog_test_ref_kfunc *p, const int size)
> +{
> +	if (size > 2 * sizeof(int))
> +		return NULL;
> +
> +	return (int *)p;
> +}
> +
> +noinline int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size)
> +{
> +	return __bpf_kfunc_call_test_get_mem(p, rdwr_buf_size);
> +}
> +
> +noinline int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size)
> +{
> +	return __bpf_kfunc_call_test_get_mem(p, rdonly_buf_size);
> +}
> +
>   noinline struct prog_test_ref_kfunc *
>   bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **pp, int a, int b)
>   {
> @@ -704,6 +722,8 @@ BTF_ID(func, bpf_kfunc_call_memb_acquire)
>   BTF_ID(func, bpf_kfunc_call_test_release)
>   BTF_ID(func, bpf_kfunc_call_memb_release)
>   BTF_ID(func, bpf_kfunc_call_memb1_release)
> +BTF_ID(func, bpf_kfunc_call_test_get_rdwr_mem)
> +BTF_ID(func, bpf_kfunc_call_test_get_rdonly_mem)
>   BTF_ID(func, bpf_kfunc_call_test_kptr_get)
>   BTF_ID(func, bpf_kfunc_call_test_pass_ctx)
>   BTF_ID(func, bpf_kfunc_call_test_pass1)
> @@ -731,6 +751,8 @@ BTF_SET_END(test_sk_release_kfunc_ids)
>   BTF_SET_START(test_sk_ret_null_kfunc_ids)
>   BTF_ID(func, bpf_kfunc_call_test_acquire)
>   BTF_ID(func, bpf_kfunc_call_memb_acquire)
> +BTF_ID(func, bpf_kfunc_call_test_get_rdwr_mem)
> +BTF_ID(func, bpf_kfunc_call_test_get_rdonly_mem)
>   BTF_ID(func, bpf_kfunc_call_test_kptr_get)
>   BTF_SET_END(test_sk_ret_null_kfunc_ids)
>   
[...]