Re: [PATCH bpf v1 01/10] bpf: Mark signal tracepoint siginfo arguments as scalar

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 4390 bytes --]

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index da36d4b9d31ab..d6d243c262ea5 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6717,6 +6717,9 @@ static const struct bpf_raw_tp_null_args raw_tp_null_args[] = {
>  	{ "rxrpc_resend", 0x10 },
>  	{ "rxrpc_tq", 0x10 },
>  	{ "rxrpc_client", 0x1 },
> +	/* signal */
> +	{ "signal_generate", 0x20 },
> +	{ "signal_deliver", 0x20 },
                                   ^^^^

Is signal_deliver's 0x20 mask too strict for its actual call sites?

Looking at the two call sites for trace_signal_deliver():

  kernel/signal.c (do_coredump):
    trace_signal_deliver(SIGKILL, SEND_SIG_NOINFO,
                         &sighand->action[SIGKILL-1]);

  kernel/signal.c (get_signal):
    trace_signal_deliver(signr, &ksig->info, ka);

SEND_SIG_PRIV is never passed to this tracepoint. SEND_SIG_NOINFO is
zero (NULL), and &ksig->info is always a real pointer filled by
dequeue_synchronous_signal() or dequeue_signal() just above the call.

SEND_SIG_PRIV == 1 only reaches trace_signal_generate() from
kernel/signal.c:__send_signal_locked().

If that's the case, would 0x10 (PTR_MAYBE_NULL) be both sufficient and
less restrictive for signal_deliver? With 0x10 the verifier would still
force a NULL check to catch SEND_SIG_NOINFO, but after the check a
tp_btf program could read info->si_code and other fields. With 0x20 the
register becomes SCALAR_VALUE immediately and check_mem_access() rejects
all dereferencing with "R1 invalid mem access 'scalar'".

This table supports per-argument distinctions. The existing entry:

  { "cachefiles_lookup", 0x1 | 0x200 }

marks arg 0 nullable and arg 2 ERR_PTR rather than applying the same
mask to both. The signal_generate justification holds (SEND_SIG_PRIV is
nonzero so PTR_MAYBE_NULL alone doesn't work), but does the same
reasoning apply to signal_deliver given that it never sees that
sentinel?

> commit a481db8aa51a8682ee36d019234a31b254b3ecd5
> Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> Date:   Thu Sep 3 16:44:19 2026 +0200
>
>     bpf: Mark signal tracepoint siginfo arguments as scalar
>
>     The signal_generate and signal_deliver tracepoints declare their info
>     argument as a struct kernel_siginfo pointer. btf_ctx_access() therefore
>     treats it as a trusted pointer for tp_btf programs.
>
>     Signal delivery also uses SEND_SIG_NOINFO and SEND_SIG_PRIV as special
>     values for this argument. Those values are zero and one respectively,
>     and are not pointers. A tp_btf program can currently dereference either
>     value and fault the kernel. In particular, signal_generate can run from
>     timer interrupt context, turning the fault into a kernel panic.
>
>     Record both tracepoints in raw_tp_null_args[] and mark argument one as
>     a non-pointer. This preserves scalar access to the cookie while rejecting
>     direct and helper-mediated pointer use. Merely marking it nullable would
>     not suffice because SEND_SIG_PRIV is nonzero.
                                                             ^^^^^^

Could this paragraph's wording be more precise about which parts are
being preserved and which argument is being marked?

"This preserves scalar access to the cookie" - there doesn't appear to
be a cookie anywhere in this change. Neither signal_generate nor
signal_deliver has a cookie argument, and this isn't related to
bpf_get_attach_cookie(). What's actually preserved is scalar access to
the siginfo sentinel value itself (0 for SEND_SIG_NOINFO, 1 for
SEND_SIG_PRIV), which is what a program can still read back. The term
"cookie" has a specific meaning in BPF context, so this might mislead
a reader trying to understand what the patch keeps working.

"mark argument one as a non-pointer" - 838a10bd2ebf documents the mask
as "1st arg: 0x1 / 2nd arg: 0x10 / 3rd arg: 0x100", where the nibble
position is the zero-indexed argument number. The mask 0x20 is nibble 1,
meaning the second argument (info). Against the referenced commit's own
wording, "argument one" could point at sig rather than info. Would
"argument 1 (info)" or "the second argument" remove the off-by-one
ambiguity?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33770534313