[PATCH v2 bpf-next 1/2] bpf: Fix Null-Pointer Dereference in kernel_clone() via BPF fmod_ret on security_task_alloc

[Feng Yang <yangfeng59949@163.com>]

[...]
> +static int modify_return_get_retval_range(const struct bpf_prog *prog,
> +					  struct bpf_retval_range *retval_range)
> +{
> +	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?

Thanks.

> +		case EI_ETYPE_ERRNO_NULL:
> +			retval_range->minval = -MAX_ERRNO;
> +			retval_range->maxval = 0;
> +			break;
> +		case EI_ETYPE_TRUE:
> +			retval_range->minval = 1;
> +			retval_range->maxval = 1;
> +			break;
> +		}
> +		retval_range->return_32bit = true;
> +
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}