On Thu, Sep 10, 2020 at 01:56:24PM +0100, Lorenz Bauer wrote:
> Function prototypes using ARG_PTR_TO_BTF_ID currently use two ways to signal
> which BTF IDs are acceptable. First, bpf_func_proto.btf_id is an array of
> IDs, one for each argument. This array is only accessed up to the highest
> numbered argument that uses ARG_PTR_TO_BTF_ID and may therefore be less than
> five arguments long. It usually points at a BTF_ID_LIST. Second, check_btf_id
> is a function pointer that is called by the verifier if present. It gets the
> actual BTF ID of the register, and the argument number we're currently checking.
> It turns out that the only user check_arg_btf_id ignores the argument, and is
> simply used to check whether the BTF ID has a struct sock_common at it's start.
>
> Replace both of these mechanisms with an explicit BTF ID for each argument
> in a function proto. Thanks to btf_struct_ids_match this is very flexible:
> check_arg_btf_id can be replaced by requiring struct sock_common.
>
[ ... ]
> @@ -4002,29 +4001,23 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
> goto err_type;
> }
> } else if (arg_type == ARG_PTR_TO_BTF_ID) {
> - bool ids_match = false;
> + const u32 *btf_id = fn->arg_btf_id[arg];
>
> expected_type = PTR_TO_BTF_ID;
> if (type != expected_type)
> goto err_type;
> - if (!fn->check_btf_id) {
> - if (reg->btf_id != meta->btf_id) {
> - ids_match = btf_struct_ids_match(&env->log, reg->off, reg->btf_id,
> - meta->btf_id);
> - if (!ids_match) {
> - verbose(env, "Helper has type %s got %s in R%d\n",
> - kernel_type_name(meta->btf_id),
> - kernel_type_name(reg->btf_id), regno);
> - return -EACCES;
> - }
> - }
> - } else if (!fn->check_btf_id(reg->btf_id, arg)) {
> - verbose(env, "Helper does not support %s in R%d\n",
> - kernel_type_name(reg->btf_id), regno);
>
> + if (!btf_id) {
> + verbose(env, "verifier internal error: missing BTF ID\n");
> + return -EFAULT;
> + }
> +
> + if (!btf_struct_ids_match(&env->log, reg->off, reg->btf_id, *btf_id)) {
> + verbose(env, "R%d has incompatible type %s\n", regno,
> + kernel_type_name(reg->btf_id));
The original log has the expected kernel type also which will be a useful info.
It could be a follow up.
Acked-by: Martin KaFai Lau <kafai@xxxxxx>
