Re: [PATCH bpf-next v5 1/6] bpf: Add syscall common attrs support for link_create

[Leon Hwang <leon.hwang@linux.dev>]

On 27/10/26 00:06, sashiko-bot@kernel.org 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?
>