From: Song Liu <songliubraving@fb.com>
To: Alexei Starovoitov <ast@kernel.org>
Cc: David Miller <davem@davemloft.net>,
"daniel@iogearbox.net" <daniel@iogearbox.net>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
Kernel Team <Kernel-team@fb.com>
Subject: Re: [PATCH v2 bpf-next 3/7] bpf: Introduce function-by-function verification
Date: Thu, 9 Jan 2020 18:09:08 +0000 [thread overview]
Message-ID: <B7A2A8DD-B070-4F80-A9A0-6570260D4346@fb.com> (raw)
In-Reply-To: <20200109063745.3154913-4-ast@kernel.org>
> On Jan 8, 2020, at 10:37 PM, Alexei Starovoitov <ast@kernel.org> wrote:
[...]
>
> Note that the stack limit of 512 still applies to the call chain regardless whether
> functions were static or global. The nested level of 8 also still applies. The
> same recursion prevention checks are in place as well.
>
> The type information and static/global kind is preserved after the verification
> hence in the above example global function f2() and f3() can be replaced later
> by equivalent functions with the same types that are loaded and verified later
> without affecting safety of this main() program. Such replacement (re-linking)
> of global functions is a subject of future patches.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Song Liu <songliubraving@fb.com>
With one nit below.
[...]
> +
> +static int do_check_common(struct bpf_verifier_env *env, int subprog)
> +{
> + struct bpf_verifier_state *state;
> + struct bpf_reg_state *regs;
> + int ret, i;
> +
> + env->prev_linfo = NULL;
> + env->pass_cnt++;
> +
> + state = kzalloc(sizeof(struct bpf_verifier_state), GFP_KERNEL);
> + if (!state)
> + return -ENOMEM;
> + state->curframe = 0;
> + state->speculative = false;
> + state->branches = 1;
> + state->frame[0] = kzalloc(sizeof(struct bpf_func_state), GFP_KERNEL);
> + if (!state->frame[0]) {
> + kfree(state);
> + return -ENOMEM;
> + }
> + env->cur_state = state;
> + init_func_state(env, state->frame[0],
> + BPF_MAIN_FUNC /* callsite */,
> + 0 /* frameno */,
> + subprog);
> +
> + regs = state->frame[state->curframe]->regs;
> + if (subprog) {
> + ret = btf_prepare_func_args(env, subprog, regs);
> + if (ret)
> + goto out;
> + for (i = BPF_REG_1; i <= BPF_REG_5; i++) {
> + if (regs[i].type == PTR_TO_CTX)
> + mark_reg_known_zero(env, regs, i);
> + else if (regs[i].type == SCALAR_VALUE)
> + mark_reg_unknown(env, regs, i);
> + }
> + } else {
> + /* 1st arg to a function */
> + regs[BPF_REG_1].type = PTR_TO_CTX;
> + mark_reg_known_zero(env, regs, BPF_REG_1);
> + ret = btf_check_func_arg_match(env, subprog, regs);
> + if (ret == -EFAULT)
> + /* unlikely verifier bug. abort.
> + * ret == 0 and ret < 0 are sadly acceptable for
> + * main() function due to backward compatibility.
> + * Like socket filter program may be written as:
> + * int bpf_prog(struct pt_regs *ctx)
> + * and never dereference that ctx in the program.
> + * 'struct pt_regs' is a type mismatch for socket
> + * filter that should be using 'struct __sk_buff'.
> + */
> + goto out;
> + }
> +
> + ret = do_check(env);
> +out:
> + if (env->cur_state) {
I think env->cur_state will never be NULL here. This check is necessary
before this patch (when we allocate cur_state in do_check()).
Thanks,
Song
next prev parent reply other threads:[~2020-01-09 18:09 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-09 6:37 [PATCH v2 bpf-next 0/7] bpf: Introduce global functions Alexei Starovoitov
2020-01-09 6:37 ` [PATCH v2 bpf-next 1/7] libbpf: Sanitize " Alexei Starovoitov
2020-01-09 6:37 ` [PATCH v2 bpf-next 2/7] libbpf: Collect static vs global info about functions Alexei Starovoitov
2020-01-09 6:37 ` [PATCH v2 bpf-next 3/7] bpf: Introduce function-by-function verification Alexei Starovoitov
2020-01-09 18:09 ` Song Liu [this message]
2020-01-09 22:17 ` Alexei Starovoitov
2020-01-22 2:30 ` Alexei Starovoitov
2020-01-09 6:37 ` [PATCH v2 bpf-next 4/7] selftests/bpf: Add fexit-to-skb test for global funcs Alexei Starovoitov
2020-01-09 6:37 ` [PATCH v2 bpf-next 5/7] selftests/bpf: Add a test for a large global function Alexei Starovoitov
2020-01-09 6:37 ` [PATCH v2 bpf-next 6/7] selftests/bpf: Modify a test to check global functions Alexei Starovoitov
2020-01-09 6:37 ` [PATCH v2 bpf-next 7/7] selftests/bpf: Add unit tests for " Alexei Starovoitov
2020-01-09 17:27 ` Song Liu
2020-01-09 22:09 ` Alexei Starovoitov
2020-01-09 22:35 ` Song Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=B7A2A8DD-B070-4F80-A9A0-6570260D4346@fb.com \
--to=songliubraving@fb.com \
--cc=Kernel-team@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).