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?
> 


Reply via email to