Re: [PATCH bpf-next v1 2/3] bpf: Emit verifier warnings through prog stderr

Puranjay Mohan
2026-03-30T12:39:49Z

Mykyta Yatsenko <mykyta.yatsenko5@gmail.com> writes:

> Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:
>
>> There is a class of messages that aren't treated as hard errors, such
>> that the program must be rejected, but should be surfaced to the user to
>> make them aware that while the program succeeded, parts of their program
>> are exhibiting behavior that needs attention. One example is usage of
>> kfuncs that are supposed to be deprecated, and may be dropped in the
>> future, though are preserved for now for backwards compatibility.
>>
>> Add support for emitting a warning message to the BPF program's stderr
>> stream whenever we detect usage of a _impl suffixed kfunc, which have
>> now been replaced with KF_IMPLICIT_ARGS variants. For this purpose,
>> introduce bpf_stream_pr_warn() as a convenience wrapper, and then mark
>> bpf_find_linfo() as global to allow its usage in the verifier to find
>> the linfo corresponding to an instruction index.
>>
>> To make the message more helpful, recommend usage of bpf_ksym_exists()
>> as a way for the user to write backwards compatible BPF code that works
>> on older kernels offering _impl suffixed kfuncs.
>>
>> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>> ---
> In the commit message: ..such that the program must __not__ be rejected..
>> include/linux/bpf.h | 9 ++++++
>> include/linux/bpf_verifier.h | 3 +-
>> kernel/bpf/log.c | 6 ++--
>> kernel/bpf/verifier.c | 53 ++++++++++++++++++++++++++++++++++++
>> 4 files changed, 67 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 2c4f92085d79..d0158edd27b2 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -3912,6 +3912,15 @@ int bpf_stream_stage_dump_stack(struct bpf_stream_stage *ss);
>> bpf_stream_stage_free(&ss); \
>> })
>>
>> +#define bpf_stream_pr_warn(prog, fmt, ...) \
>> + ({ \
>> + struct bpf_stream_stage __ss; \
>> + \
>> + bpf_stream_stage(__ss, prog, BPF_STDERR, ({ \
>> + bpf_stream_printk(__ss, fmt, ##__VA_ARGS__); \
>> + })); \
>> + })
>> +
>> #ifdef CONFIG_BPF_LSM
>> void bpf_cgroup_atype_get(u32 attach_btf_id, int cgroup_atype);
>> void bpf_cgroup_atype_put(int cgroup_atype);
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index 090aa26d1c98..5683c06f5a90 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -873,7 +873,8 @@ int bpf_vlog_init(struct bpf_verifier_log *log, u32 log_level,
>> char __user *log_buf, u32 log_size);
>> void bpf_vlog_reset(struct bpf_verifier_log *log, u64 new_pos);
>> int bpf_vlog_finalize(struct bpf_verifier_log *log, u32 *log_size_actual);
>> -
>> +const struct bpf_line_info *bpf_find_linfo(const struct bpf_verifier_env *env,
>> + u32 insn_off);
>> __printf(3, 4) void verbose_linfo(struct bpf_verifier_env *env,
>> u32 insn_off,
>> const char *prefix_fmt, ...);
>> diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
>> index 37d72b052192..598b494ded36 100644
>> --- a/kernel/bpf/log.c
>> +++ b/kernel/bpf/log.c
>> @@ -329,8 +329,8 @@ __printf(2, 3) void bpf_log(struct bpf_verifier_log *log,
>> }
>> EXPORT_SYMBOL_GPL(bpf_log);
>>
>> -static const struct bpf_line_info *
>> -find_linfo(const struct bpf_verifier_env *env, u32 insn_off)
>> +const struct bpf_line_info *
>> +bpf_find_linfo(const struct bpf_verifier_env *env, u32 insn_off)
>> {
>> const struct bpf_line_info *linfo;
>> const struct bpf_prog *prog;
>> @@ -390,7 +390,7 @@ __printf(3, 4) void verbose_linfo(struct bpf_verifier_env *env,
>> return;
>>
>> prev_linfo = env->prev_linfo;
>> - linfo = find_linfo(env, insn_off);
>> + linfo = bpf_find_linfo(env, insn_off);
>> if (!linfo || linfo == prev_linfo)
>> return;
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 8c1cf2eb6cbb..f50e0ebd0ded 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -3206,6 +3206,7 @@ struct bpf_kfunc_desc {
>> u32 func_id;
>> s32 imm;
>> u16 offset;
>> + bool warned_deprecated;
>> unsigned long addr;
>> };
>>
>> @@ -14045,6 +14046,53 @@ static int fetch_kfunc_arg_meta(struct bpf_verifier_env *env,
>> return 0;
>> }
>>
>> +static bool get_insn_file_line(const struct bpf_verifier_env *env, u32 insn_off,
>> + const char **filep, int *linep)
>> +{
>> + const struct bpf_line_info *linfo;
>> +
>> + if (!env->prog->aux->btf)
>> + return false;
>> +
>> + linfo = bpf_find_linfo(env, insn_off);
>> + if (!linfo)
>> + return false;
>> +
>> + if (bpf_get_linfo_file_line(env->prog->aux->btf, linfo, filep, NULL, linep))
>> + return false;
>> + return true;
>> +}
>> +
>> +static void warn_for_deprecated_kfuncs(struct bpf_verifier_env *env,
>> + struct bpf_kfunc_desc *desc,
>> + const char *func_name,
>> + int insn_idx)
>> +{
>> + int repl_len, line;
>> + const char *file;
>> +
>> + if (desc->warned_deprecated)
>> + return;
>> +
>> + if (!func_name || !strends(func_name, KF_IMPL_SUFFIX))
>> + return;
>> +
>> + repl_len = strlen(func_name) - strlen(KF_IMPL_SUFFIX);
>> +
>> + if (get_insn_file_line(env, insn_idx, &file, &line))
>> + snprintf(env->tmp_str_buf, TMP_STR_BUF_LEN, "%s:%u", file, line);
>> + else
>> + snprintf(env->tmp_str_buf, TMP_STR_BUF_LEN, "insn #%d", insn_idx);
>> +
>> + bpf_stream_pr_warn(env->prog,
>> + "WARNING: %s calls deprecated kfunc %s(), which will be removed.\n"
>> + "WARNING: Switch to kfunc %.*s() instead.\n"
>> + "WARNING: For older kernels, choose the kfunc using bpf_ksym_exists(%.*s).\n",
>> + env->tmp_str_buf, func_name, repl_len, func_name, repl_len, func_name);
>> +
>> + desc->warned_deprecated = true;
>> +}
> Commit message suggests this is a generic functionality that we plan to
> use for all kinds of deprecations, but this function looks tightly
> coupled to KF_IMPL_SUFFIX functions. I think we can make it a bit
> friendlier to extending if split into 2 separate helpers:
> is_kfunc_deprecated() and warn_deprecated_kfunc(), so in future new
> logic for detecting deprecated funcs goes straight to
> is_kfunc_deprecated() and we make sure printing and file/line
> extractions are not changed.

I agree with splitting this into two separate helpers.