[bpf-next] bpf: emit better log message if bpf_iter ctx arg btf_id == 0

Commit Message

Yonghong Song July 28, 2021, 6:30 p.m. UTC

To avoid kernel build failure due to some missing .BTF-ids referenced
functions/types, the patch ([1]) tries to fill btf_id 0 for
these types.

In bpf verifier, for percpu variable and helper returning btf_id cases,
verifier already emitted proper warning with something like
  verbose(env, "Helper has invalid btf_id in R%d\n", regno);
  verbose(env, "invalid return type %d of func %s#%d\n",
          fn->ret_type, func_id_name(func_id), func_id);

But this is not the case for bpf_iter context arguments.
I hacked resolve_btfids to encode btf_id 0 for struct task_struct.
With `./test_progs -n 7/5`, I got,
  0: (79) r2 = *(u64 *)(r1 +0)
  func 'bpf_iter_task' arg0 has btf_id 29739 type STRUCT 'bpf_iter_meta'
  ; struct seq_file *seq = ctx->meta->seq;
  1: (79) r6 = *(u64 *)(r2 +0)
  ; struct task_struct *task = ctx->task;
  2: (79) r7 = *(u64 *)(r1 +8)
  ; if (task == (void *)0) {
  3: (55) if r7 != 0x0 goto pc+11
  ...
  ; BPF_SEQ_PRINTF(seq, "%8d %8d\n", task->tgid, task->pid);
  26: (61) r1 = *(u32 *)(r7 +1372)
  Type '(anon)' is not a struct

Basically, verifier will return btf_id 0 for task_struct.
Later on, when the code tries to access task->tgid, the
verifier correctly complains the type is '(anon)' and it is
not a struct. Users still need to backtrace to find out
what is going on.

Let us catch the invalid btf_id 0 earlier
and provide better message indicating btf_id is wrong.
The new error message looks like below:
  R1 type=ctx expected=fp
  ; struct seq_file *seq = ctx->meta->seq;
  0: (79) r2 = *(u64 *)(r1 +0)
  func 'bpf_iter_task' arg0 has btf_id 29739 type STRUCT 'bpf_iter_meta'
  ; struct seq_file *seq = ctx->meta->seq;
  1: (79) r6 = *(u64 *)(r2 +0)
  ; struct task_struct *task = ctx->task;
  2: (79) r7 = *(u64 *)(r1 +8)
  invalid btf_id for context argument offset 8
  invalid bpf_context access off=8 size=8

[1] https://lore.kernel.org/bpf/[e-mail]/

Signed-off-by: Yonghong Song <[e-mail]>
---
 kernel/bpf/btf.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

[e-mail] July 29, 2021, 10:20 p.m. UTC | #1

Hello:

This patch was applied to bpf/bpf-next.git (refs/heads/master):

On Wed, 28 Jul 2021 11:30:25 -0700 you wrote:
> To avoid kernel build failure due to some missing .BTF-ids referenced
> functions/types, the patch ([1]) tries to fill btf_id 0 for
> these types.
> 
> In bpf verifier, for percpu variable and helper returning btf_id cases,
> verifier already emitted proper warning with something like
>   verbose(env, "Helper has invalid btf_id in R%d\n", regno);
>   verbose(env, "invalid return type %d of func %s#%d\n",
>           fn->ret_type, func_id_name(func_id), func_id);
> 
> [...]

Here is the summary with links:
  - [bpf-next] bpf: emit better log message if bpf_iter ctx arg btf_id == 0
    https://git.kernel.org/bpf/bpf-next/c/d36216429ff3

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Patch

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 7780131f710e..c395024610ed 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4825,6 +4825,11 @@  bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 		const struct bpf_ctx_arg_aux *ctx_arg_info = &prog->aux->ctx_arg_info[i];
 
 		if (ctx_arg_info->offset == off) {
+			if (!ctx_arg_info->btf_id) {
+				bpf_log(log,"invalid btf_id for context argument offset %u\n", off);
+				return false;
+			}
+
 			info->reg_type = ctx_arg_info->reg_type;
 			info->btf = btf_vmlinux;
 			info->btf_id = ctx_arg_info->btf_id;