2021-09-02 10:11:10

by Jiasheng Jiang

Subject: [PATCH 1/3] bpf: Add env_type_is_resolved() in front of env_stack_push() in btf_resolve()

From: jiasheng <[email protected]>

We have found that in the complied files env_stack_push()
appear more than 10 times, and under at least 90% circumstances
that env_type_is_resolved() and env_stack_push() appear in pairs.
For example, they appear together in the btf_modifier_resolve()
of the file complie from 'kernel/bpf/btf.c'.
But we have found that in the btf_resolve(), there is only
env_stack_push() instead of the pair.
Therefore, we consider that the env_type_is_resolved()
might be forgotten.

Signed-off-by: jiasheng <[email protected]>
---
kernel/bpf/btf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index f982a9f0..454c249 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4002,7 +4002,8 @@ static int btf_resolve(struct btf_verifier_env *env,
int err = 0;

env->resolve_mode = RESOLVE_TBD;
- env_stack_push(env, t, type_id);
+ if (env_type_is_resolved(env, type_id))
+ env_stack_push(env, t, type_id);
while (!err && (v = env_stack_peak(env))) {
env->log_type_id = v->type_id;
err = btf_type_ops(v->t)->resolve(env, v);
--
2.7.4

2021-09-02 10:53:38

by Borislav Petkov

Subject: Re: [PATCH 1/3] bpf: Add env_type_is_resolved() in front of env_stack_push() in btf_resolve()

On Thu, Sep 02, 2021 at 10:04:57AM +0000, Jiang Jiasheng wrote:
> From: jiasheng <[email protected]>
>
> We have found that in the complied files env_stack_push()
> appear more than 10 times, and under at least 90% circumstances
> that env_type_is_resolved() and env_stack_push() appear in pairs.
> For example, they appear together in the btf_modifier_resolve()
> of the file complie from 'kernel/bpf/btf.c'.
> But we have found that in the btf_resolve(), there is only
> env_stack_push() instead of the pair.
> Therefore, we consider that the env_type_is_resolved()
> might be forgotten.
>
> Signed-off-by: jiasheng <[email protected]>
> ---
> kernel/bpf/btf.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index f982a9f0..454c249 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -4002,7 +4002,8 @@ static int btf_resolve(struct btf_verifier_env *env,
> int err = 0;
>
> env->resolve_mode = RESOLVE_TBD;
> - env_stack_push(env, t, type_id);
> + if (env_type_is_resolved(env, type_id))
> + env_stack_push(env, t, type_id);
> while (!err && (v = env_stack_peak(env))) {
> env->log_type_id = v->type_id;
> err = btf_type_ops(v->t)->resolve(env, v);
> --

You still didn't read what I suggested, did you?

Because there it also explains who to CC on patches. Like, for example,
for this wrong patch you've CCed the wrong people.

Oh well, until you do, I'm deleting your mail.

Good luck.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette