linux-kernel - Re: [PATCH bpf-next v8 0/5] bpf: Add open-coded style process file iterator and bpf_fget

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] [day] [month] [year] [list]

Message-ID:
 <AM6PR03MB5080DFE11B733C9DCD6547E199EC2@AM6PR03MB5080.eurprd03.prod.outlook.com>
Date: Mon, 27 Jan 2025 22:23:06 +0000
From: Juntong Deng <juntong.deng@...look.com>
To: ast@...nel.org, daniel@...earbox.net, john.fastabend@...il.com,
 andrii@...nel.org, martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org,
 yonghong.song@...ux.dev, kpsingh@...nel.org, sdf@...ichev.me,
 haoluo@...gle.com, jolsa@...nel.org, memxor@...il.com, snorcht@...il.com,
 brauner@...nel.org
Cc: bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH bpf-next v8 0/5] bpf: Add open-coded style process file
 iterator and bpf_fget_task() kfunc

I noticed an interesting problem in Kernel CI.

For gcc-compiled kernel (x86_64-gcc, aarch64-gcc, s390x-gcc), the
iters/task_file test case can pass.

But for llvm-compiled kernel (x86_64-llvm-17, x86_64-llvm-18), the
iters/task_file test case fails.
The following is the error:

; if (task->parent->pid != parent_pid) @ iters_task_file.c:26
1: (79) r1 = *(u64 *)(r0 +1696)       ; R0_w=trusted_ptr_task_struct()
R1_w=rcu_ptr_or_null_task_struct(id=1)
2: (61) r1 = *(u32 *)(r1 +1672)
R1 invalid mem access 'rcu_ptr_or_null_'

I reproduced this error on my local laptop. With the same ebpf program,
it can be loaded normally into gcc-compiled kernel, but loading it into
llvm-compiled (make LLVM=1) kernel will result in an error.

The behavior of the gcc-compiled kernel is inconsistent with the
llvm-compiled kernel.

After my analysis, this is because of the following reasons:

Currently, llvm-compiled kernel can determine whether a pointer is rcu
or rcu_or_null based on btf_type_tag and allowlist, but gcc-compiled
kernel can only based on allowlist, because gcc does not support
btf_type_tag (commit 6fcd486b3a0a "bpf: Refactor RCU enforcement
in the verifier.").

Since the verifier cannot determine which fields are safe to read, it is
not feasible to set all untagged fields to PTR_UNTRUSTED, so just remove
the PTR_TRUSTED (commit afeebf9f57a4 "bpf: Undo strict enforcement for
walking untagged fields.").

These end up leading to an interesting problem.

static int check_ptr_to_btf_access(...)
{
...
    } else if (in_rcu_cs(env) && !type_may_be_null(reg->type)) {
       if (type_is_rcu(env, reg, field_name, btf_id)) {   (1)
          /* ignore __rcu tag and mark it MEM_RCU */
          flag |= MEM_RCU;
       } else if (flag & MEM_RCU ||                       (2)
          type_is_rcu_or_null(env, reg, field_name, btf_id)) {
          /* __rcu tagged pointers can be NULL */
          flag |= MEM_RCU | PTR_MAYBE_NULL;

          /* We always trust them */
          if (type_is_rcu_or_null(env, reg, field_name, btf_id) &&
             flag & PTR_UNTRUSTED)
                flag &= ~PTR_UNTRUSTED;
       } else if (flag & (MEM_PERCPU | MEM_USER)) {       (3)
          /* keep as-is */
       } else {                                           (4)
       /* walking unknown pointers yields old deprecated PTR_TO_BTF_ID */
          clear_trusted_flags(&flag);
       }
...
}

In gcc-compiled kernel, since task->parent is not in BTF_TYPE_SAFE_RCU
and BTF_TYPE_SAFE_RCU_OR_NULL, and since btf_type_tag is not supported,
there is no MEM_RCU, branch (4) is reached, just remove PTR_TRUSTED.
Although we cannot pass such a pointer to a kfunc with KF_TRUSTED_ARGS
or KF_RCU, we can still dereference it.

In llvm-compiled kernel, since task->parent is tagged with __rcu,
branch (2) is reached and flag is set to MEM_RCU | PTR_MAYBE_NULL.
This results in the "R1 invalid mem access rcu_ptr_or_null_" error
mentioned earlier.

If we remove the __rcu tag of task->parent, then there is no error,
even with the llvm-compiled kernel, because branch (4) is reached like
the gcc-compiled kernel.

This leads to the interesting result that pointers not tagged with __rcu
will have more freedom to dereference than pointers tagged with __rcu.

This result is obviously incorrect because the pointer may be NULL
whether or not it is tagged with __rcu.

But how to fix this problem seems a bit tricky.

It is not possible to mark every data structure member whether it may
be NULL.

If every pointer has PTR_MAYBE_NULL by default, chained dereferencing
will become cumbersome.

More discussion is welcome.

Many thanks.