Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220202211328.176481-1-mcroce@linux.microsoft.com> <20220202211328.176481-3-mcroce@linux.microsoft.com>
In-Reply-To: <20220202211328.176481-3-mcroce@linux.microsoft.com>
From:   Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Thu, 3 Feb 2022 09:36:38 -0800
Message-ID: <CAADnVQ+PpCHnQoTxJ2V0305SnSmftngyGGaq1m71ai0KS1jZ9w@mail.gmail.com>
Subject: Re: [PATCH bpf-next v2 2/2] selftests/bpf: test maximum recursion
 depth for bpf_core_types_are_compat()
To:     Matteo Croce <mcroce@linux.microsoft.com>
Cc:     bpf <bpf@vger.kernel.org>, Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Feb 2, 2022 at 1:13 PM Matteo Croce <mcroce@linux.microsoft.com> wrote:
>
> From: Matteo Croce <mcroce@microsoft.com>
>
> bpf_core_types_are_compat() was limited to 2 recursion levels, which are
> enough to parse a function prototype.
> Add a test which checks the existence of a function prototype, so to
> test the bpf_core_types_are_compat() code path.
>
> Signed-off-by: Matteo Croce <mcroce@microsoft.com>
> ---
>  .../selftests/bpf/bpf_testmod/bpf_testmod.c        |  3 +++
>  tools/testing/selftests/bpf/progs/core_kern.c      | 14 ++++++++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> index 595d32ab285a..a457071a7751 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> @@ -13,6 +13,9 @@
>  #define CREATE_TRACE_POINTS
>  #include "bpf_testmod-events.h"
>
> +typedef int (*func_proto_typedef)(long);
> +func_proto_typedef funcp = NULL;
> +
>  DEFINE_PER_CPU(int, bpf_testmod_ksym_percpu) = 123;
>
>  noinline void
> diff --git a/tools/testing/selftests/bpf/progs/core_kern.c b/tools/testing/selftests/bpf/progs/core_kern.c
> index 13499cc15c7d..bfea86b42563 100644
> --- a/tools/testing/selftests/bpf/progs/core_kern.c
> +++ b/tools/testing/selftests/bpf/progs/core_kern.c
> @@ -101,4 +101,18 @@ int balancer_ingress(struct __sk_buff *ctx)
>         return 0;
>  }
>
> +typedef int (*func_proto_typedef___match)(long);
> +typedef void (*func_proto_typedef___doesnt_match)(char*);
> +
> +int out[2];
> +
> +SEC("raw_tracepoint/sys_enter")
> +int core_relo_recur_limit(void *ctx)
> +{
> +       out[0] = bpf_core_type_exists(func_proto_typedef___match);
> +       out[1] = bpf_core_type_exists(func_proto_typedef___doesnt_match);

How does it test it?
The kernel code could be a nop and there will be no failure in this "test".
Please make it real.
Also add tests that exercise the limit of recursion.
One that goes over and fails and another that is right at the limit
and passes.