From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel Borkmann <daniel@iogearbox.net>
Subject: Re: [PATCH v4 bpf-next 08/14] bpf: introduce the
 bpf_get_local_storage() helper function
Date: Wed, 1 Aug 2018 00:50:16 +0200
Message-ID: <ad6c58e3-08a8-aa9c-2d69-528007fe126e@iogearbox.net>
References: <20180727215243.3850-1-guro@fb.com>
 <20180727215243.3850-9-guro@fb.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: linux-kernel@vger.kernel.org, kernel-team@fb.com,
        Alexei Starovoitov <ast@kernel.org>
To: Roman Gushchin <guro@fb.com>, netdev@vger.kernel.org
Return-path: <linux-kernel-owner@vger.kernel.org>
In-Reply-To: <20180727215243.3850-9-guro@fb.com>
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
List-Id: netdev.vger.kernel.org

On 07/27/2018 11:52 PM, Roman Gushchin wrote:
[...]
> @@ -2533,6 +2541,16 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
>  	}
>  
>  	regs = cur_regs(env);
> +
> +	/* check that flags argument in get_local_storage(map, flags) is 0,
> +	 * this is required because get_local_storage() can't return an error.
> +	 */
> +	if (func_id == BPF_FUNC_get_local_storage &&
> +	    !tnum_equals_const(regs[BPF_REG_2].var_off, 0)) {
> +		verbose(env, "get_local_storage() doesn't support non-zero flags\n");
> +		return -EINVAL;
> +	}

Hmm, this check is actually not correct. You will still be able to pass non-zero
values in there. arg2_type from the helper is ARG_ANYTHING, so the register type
could for example be one of the pointer types and it will still pass the verifier.
The correct way to check would be to use register_is_null().

> +
>  	/* reset caller saved regs */
>  	for (i = 0; i < CALLER_SAVED_REGS; i++) {
>  		mark_reg_not_init(env, regs, caller_saved[i]);