On 27/10/26 00:06, [email protected] wrote:
[...]
>>
>> switch (prog->type) {
>> case BPF_PROG_TYPE_CGROUP_SKB:
>> @@ -5959,12 +5972,20 @@ static int link_create(union bpf_attr *attr,
>> bpfptr_t uattr)
>> ret = bpf_uprobe_multi_link_attach(attr, prog);
>> break;
>> default:
>> + bpf_log(log, "Invalid prog type.\n");
>> ret = -EINVAL;
>> }
>
> [Severity: Medium]
> Does calling bpf_prog_attach_check_attach_type() before switch (prog->type)
> in kernel/bpf/syscall.c:link_create() cause misleading or missing log
> messages in a few cases?
>
> First, for almost all program types that are unsupported by link_create()
> (such as BPF_PROG_TYPE_SOCKET_FILTER, BPF_PROG_TYPE_SCHED_ACT,
> BPF_PROG_TYPE_RAW_TRACEPOINT, or BPF_PROG_TYPE_SYSCALL),
> link_create() -> bpf_prog_attach_check_attach_type() hits the default
> branch:
>
> kernel/bpf/syscall.c:bpf_prog_attach_check_attach_type() {
> ...
> default:
> ptype = attach_type_to_prog_type(attach_type);
> if (ptype == BPF_PROG_TYPE_UNSPEC || ptype != prog->type)
> return -EINVAL;
> return 0;
> }
>
> Because attach_type_to_prog_type() never returns those program types for any
> attach_type, bpf_prog_attach_check_attach_type() returns -EINVAL and
> link_create() logs "Invalid attach_type.\n" instead of reaching the default
> branch of switch (prog->type) to log "Invalid prog type.\n".
Either "Invalid attach_type.\n" or "Invalid prog type.\n" is not
accurate for this case.
The log here is not required to accurately describe the reason. Instead,
the log tells users something wrong at the position.
>
> Second, for BPF_PROG_TYPE_CGROUP_SKB, bpf_prog_attach_check_attach_type()
> also performs a capability check:
>
> case BPF_PROG_TYPE_CGROUP_SKB:
> if (!bpf_token_capable(prog->aux->token, CAP_NET_ADMIN))
> return -EPERM;
>
> When this check fails with -EPERM, link_create() still logs
> "Invalid attach_type.\n" even when attach_type is valid.
Same above.
>
> Third, for BPF_PROG_TYPE_LSM and BPF_PROG_TYPE_TRACING,
> bpf_prog_attach_check_attach_type() only checks that attach_type maps to the
> general program type. The stricter expected_attach_type check happens inside
> switch (prog->type) in link_create():
>
> case BPF_PROG_TYPE_LSM:
> case BPF_PROG_TYPE_TRACING:
> if (attr->link_create.attach_type !=
> prog->expected_attach_type) {
> ret = -EINVAL;
> goto out;
> }
>
> If a caller passes an attach_type of the same program type that does not
> match prog->expected_attach_type, link_create() jumps to out without logging
> "Invalid attach_type.\n".
Will add a log here when adding log support for tracing link_create in
the future.
Thanks,
Leon
>
> Should the program type validation and attach_type logging be ordered so
> that unsupported program types, -EPERM failures, and expected_attach_type
> mismatches report accurate messages?
>