Re: [PATCH bpf 1/2] bpf: Reject >8 byte return values on return-reading trampoline paths

[Yonghong Song <yonghong.song@linux.dev>]

On 7/10/26 9:37 AM, Leon Hwang wrote:
> On 2026/7/10 22:44, Yonghong Song wrote:
>
> [...]
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 6515d4d3c003..26d281b77a6e 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -18873,6 +18873,20 @@ static int btf_id_allow_sleepable(u32 btf_id, unsigned long addr, const struct b
>>   	return -EINVAL;
>>   }
>>   
>> +static bool attach_uses_trampoline_retval(enum bpf_attach_type type)
>> +{
>> +	switch (type) {
>> +	case BPF_MODIFY_RETURN:
>> +	case BPF_TRACE_FEXIT:
>> +	case BPF_TRACE_FEXIT_MULTI:
>> +	case BPF_TRACE_FSESSION:
>> +	case BPF_TRACE_FSESSION_MULTI:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
>> +
>>   int bpf_check_attach_target(struct bpf_verifier_log *log,
>>   			    const struct bpf_prog *prog,
>>   			    const struct bpf_prog *tgt_prog,
>> @@ -19137,6 +19151,14 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
>>   		if (ret < 0)
>>   			return ret;
>>   
>> +		if (tgt_info->fmodel.ret_size > 8 &&
>> +		    attach_uses_trampoline_retval(prog->expected_attach_type)) {
>> +			bpf_log(log,
>> +				"Attach to function %s with a >8 byte return value is not supported for this attach type\n",
>> +				tname);
>> +			return -EOPNOTSUPP;
>> +		}
>> +
>>   		/*
>>   		 * *.multi programs don't need an address during program
>>   		 * verification, we just take the module ref if needed.
>> @@ -19413,6 +19435,9 @@ int bpf_check_attach_btf_id_multi(struct btf *btf, struct bpf_prog *prog, u32 bt
>>   	err = btf_distill_func_proto(NULL, btf, t, tname, &tgt_info->fmodel);
>>   	if (err < 0)
>>   		return err;
>> +	if (tgt_info->fmodel.ret_size > 8 &&
>> +	    attach_uses_trampoline_retval(prog->expected_attach_type))
>> +		return -EOPNOTSUPP;
>>   	if (btf_is_module(btf)) {
>>   		/* The bpf program already holds reference to module. */
>>   		if (WARN_ON_ONCE(!prog->aux->mod))
>
> I think there's no need to introduce the helper
> attach_uses_trampoline_retval().
>
> See this untested diff:
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 03e2202cca13..899d5b129b54 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -19023,6 +19023,7 @@ int bpf_check_attach_target(struct
> bpf_verifier_log *log,
>   	const char prefix[] = "btf_trace_";
>   	struct bpf_raw_event_map *btp;
>   	int ret = 0, subprog = -1, i;
> +	bool attach_retval = false;
>   	const struct btf_type *t;
>   	bool conservative = true;
>   	const char *tname, *fname;
> @@ -19232,19 +19233,21 @@ int bpf_check_attach_target(struct
> bpf_verifier_log *log,
>   		if (ret)
>   			return ret;
>   		break;
> +	case BPF_MODIFY_RETURN:
> +	case BPF_TRACE_FEXIT:
> +	case BPF_TRACE_FEXIT_MULTI:
> +	case BPF_TRACE_FSESSION:
> +	case BPF_TRACE_FSESSION_MULTI:
> +		attach_retval = true;
> +		fallthrough;
>   	default:
> -		if (!prog_extension)
> +		if (!prog_extension && !attach_retval)
>   			return -EINVAL;
>   		fallthrough;
> -	case BPF_MODIFY_RETURN:
>   	case BPF_LSM_MAC:
>   	case BPF_LSM_CGROUP:
>   	case BPF_TRACE_FENTRY:
> -	case BPF_TRACE_FEXIT:
> -	case BPF_TRACE_FSESSION:
> -	case BPF_TRACE_FSESSION_MULTI:
>   	case BPF_TRACE_FENTRY_MULTI:
> -	case BPF_TRACE_FEXIT_MULTI:
>   		if ((prog->expected_attach_type == BPF_TRACE_FSESSION ||
>   		    prog->expected_attach_type == BPF_TRACE_FSESSION_MULTI) &&
>   		    !bpf_jit_supports_fsession()) {
> @@ -19275,6 +19278,13 @@ int bpf_check_attach_target(struct

Thanks for the suggestion. I still prefers the helper approach. It provides
a single place in verifier.c to handle this case (tgt_info->fmodel.ret_size > 8).
The above switch already very subtle and the change makes it even more subtle.

> bpf_verifier_log *log,
>   		if (ret < 0)
>   			return ret;
>
> +		if (tgt_info->fmodel.ret_size > 8 && attach_retval) {
> +			bpf_log(log,
> +				"Attach to function %s with a >8 byte return value is not supported
> for this attach type\n",
> +				tname);
> +			return -EOPNOTSUPP;
> +		}
> +
>   		/*
>   		 * *.multi programs don't need an address during program
>   		 * verification, we just take the module ref if needed.
> @@ -19551,6 +19561,13 @@ int bpf_check_attach_btf_id_multi(struct btf
> *btf, struct bpf_prog *prog, u32 bt
>   	err = btf_distill_func_proto(NULL, btf, t, tname, &tgt_info->fmodel);
>   	if (err < 0)
>   		return err;
> +	switch (prog->expected_attach_type) {
> +	case BPF_TRACE_FEXIT_MULTI:
> +	case BPF_TRACE_FSESSION_MULTI:
> +		if (tgt_info->fmodel.ret_size > 8)
> +			return -EOPNOTSUPP;
> +		break;
> +	}
>   	if (btf_is_module(btf)) {
>   		/* The bpf program already holds reference to module. */
>   		if (WARN_ON_ONCE(!prog->aux->mod))
>
>
> WDYT?
>
> Thanks,
> Leon
>