Re: [PATCH bpf-next v13 2/6] bpf: add bpf_get_cpu_time_counter kfunc

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [High] Type confusion in `bpf_jit_inlines_kfunc_call` breaks fastcall optimization and hides a TOCTOU register corruption vulnerability.
- [High] The `bpf_get_cpu_time_counter()` kfunc returns inconsistent physical units (CPU cycles vs. nanoseconds) depending on JIT execution.
- [Medium] Missing memory barrier for `RDTSC` allows speculative out-of-order execution on certain CPUs.
--

commit e2ed941037816a39f712a14ccb3383436319583d
Author: Vadim Fedorenko <vadim.fedorenko@linux.dev>

bpf: add bpf_get_cpu_time_counter kfunc

This commit adds a bpf_get_cpu_time_counter kfunc to return an
ARCH-specific timecounter, bypassing the usual ktime_get_ns() overhead.
It implements an inlined x86_64 JIT version using RDTSC, and falls back
to ktime_get_raw_fast_ns() in C.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index ea9e707e8abff..7cda5589107bf 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c

[ ... ]

> @@ -2445,6 +2446,40 @@ st:			if (is_imm8(insn->off))
>  
>  			/* call */
>  		case BPF_JMP | BPF_CALL: {
> +			u8 *ip = image + addrs[i - 1];
> +
> +			if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
> +			    imm32 == BPF_CALL_IMM(bpf_get_cpu_time_counter) &&
> +			    bpf_jit_inlines_kfunc_call(imm32)) {
> +				/* The default implementation of this kfunc uses
> +				 * ktime_get_raw_ns() which effectively is implemented as
> +				 * `(u64)rdtsc_ordered() & S64_MAX`. For JIT We skip

Does this JIT implementation return inconsistent time units compared to the C
fallback?

The inline RDTSC emits hardware CPU cycles, but the fallback
bpf_get_cpu_time_counter() calls ktime_get_raw_fast_ns(), which returns
nanoseconds. If a BPF program calculates a delta using the fallback path and
later converts it, won't the nanosecond value be multiplied again, resulting
in incorrect metrics?

[ ... ]

> +				emit_mov_reg(&prog, true, AUX_REG, BPF_REG_3);
> +				if (cpu_feature_enabled(X86_FEATURE_LFENCE_RDTSC))
> +					EMIT_LFENCE();
> +				EMIT2(0x0F, 0x31);

The kernel's standard rdtsc_ordered() implementation ensures serialization by
checking both X86_FEATURE_LFENCE_RDTSC and X86_FEATURE_MFENCE_RDTSC.

Does this code need to check for MFENCE as well? On older AMD CPUs that rely
on MFENCE, omitting it could result in out-of-order execution across the
measured code block.

[ ... ]

> @@ -4081,3 +4116,13 @@ bool bpf_jit_supports_fsession(void)
>  {
>  	return true;
>  }
> +
> +/* x86-64 JIT can inline kfunc */
> +bool bpf_jit_inlines_kfunc_call(s32 imm)
> +{
> +	if (imm == BPF_CALL_IMM(bpf_get_cpu_time_counter) &&
> +	    cpu_feature_enabled(X86_FEATURE_TSC) &&
> +	    using_native_sched_clock() && sched_clock_stable())
> +		return true;
> +	return false;
> +}

Is it safe to evaluate sched_clock_stable() dynamically for fastcall
optimizations?

If sched_clock_stable() returns true during verification, the verifier
could strip the caller-saved register spills. If the clock later transitions
to false before JIT compilation, the JIT will emit a standard C call
instead of inlining, which could silently clobber BPF registers R1-R5.

[ ... ]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 69d75515ed3f7..b8d26e1bff481 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -17035,7 +17035,9 @@ bool bpf_get_call_summary(struct bpf_verifier_env *env, struct bpf_insn *call,
>  			/* error would be reported later */
>  			return false;
>  		cs->num_params = btf_type_vlen(meta.func_proto);
> -		cs->fastcall = meta.kfunc_flags & KF_FASTCALL;
> +		cs->fastcall = (meta.kfunc_flags & KF_FASTCALL) ||
> +			       (meta.btf == btf_vmlinux &&
> +				bpf_jit_inlines_kfunc_call(call->imm));
>  		cs->is_void = btf_type_is_void(btf_type_by_id(meta.btf, meta.func_proto->type));
>  		return true;
>  	}

In the verifier, call->imm holds the kfunc's BTF ID, but the x86 JIT
implementation of bpf_jit_inlines_kfunc_call() compares it against
BPF_CALL_IMM(bpf_get_cpu_time_counter), which is a relative memory offset.

Since a BTF ID will never match this memory offset, won't this always
return false during verification, defeating the fastcall optimization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260418131614.1501848-1-puranjay@kernel.org?part=2