On 10/4/26 16:03, Feng Yang wrote:
> [...]
>> +static int modify_return_get_retval_range(const struct bpf_prog *prog,
>> + struct bpf_retval_range *retval_range)
How about 'range' instead of 'retval_range' like its caller?
>> +{
>> + unsigned long addr = (unsigned
>> long)prog->aux->dst_trampoline->func.addr;
>> +
>> + if (within_error_injection_list(addr)) {
>> + switch (get_injectable_error_type(addr)) {
>> + case EI_ETYPE_NULL:
>> + retval_range->minval = 0;
>> + retval_range->maxval = 0;
>> + break;
>> + case EI_ETYPE_ERRNO:
>> + retval_range->minval = -MAX_ERRNO;
>> + retval_range->maxval = -1;
>> + break;
>
> This refers to the documentation in fault-injection.rst:
>
> Each error injectable functions will have the error type specified by the
> ALLOW_ERROR_INJECTION() macro. You have to choose it carefully if you add
> a new error injectable function. If the wrong error type is chosen, the
> kernel may crash because it may not be able to handle the error.
> There are 4 types of errors defined in include/asm-generic/error-injection.h
>
> EI_ETYPE_NULL
> This function will return `NULL` if it fails. e.g. return an allocated
> object address.
>
> EI_ETYPE_ERRNO
> This function will return an `-errno` error code if it fails. e.g. return
> -EINVAL if the input is wrong. This will include the functions which will
> return an address which encodes `-errno` by ERR_PTR() macro.
>
> EI_ETYPE_ERRNO_NULL
> This function will return an `-errno` or `NULL` if it fails. If the caller
> of this function checks the return value with IS_ERR_OR_NULL() macro, this
> type will be appropriate.
>
> EI_ETYPE_TRUE
> This function will return `true` (non-zero positive value) if it fails.
>
> Restrict EI_ETYPE_ERRNO to only return error codes.
> However, it was noticed that the self-test bpf_testmod_test_read uses
> ALLOW_ERROR_INJECTION(bpf_testmod_test_read, ERRNO); and returns 0, which
> causes a failure.
> So should returning 0 be considered valid for the EI_ETYPE_ERRNO type, or
> should the self-test be modified instead?
>
I think it would be better to add such explanation in code comment.
Thanks,
Leon
