From: Daniel Borkmann <daniel@iogearbox.net>
To: Yonghong Song <yhs@fb.com>, bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
kernel-team@fb.com, Song Liu <songliubraving@fb.com>
Subject: Re: [PATCH bpf-next v3 1/2] bpf: permits pointers on stack for helper calls
Date: Mon, 14 Dec 2020 22:26:43 +0100 [thread overview]
Message-ID: <e48545be-6b03-aa2b-d5f6-a12b180ba116@iogearbox.net> (raw)
In-Reply-To: <20201211034121.3452243-1-yhs@fb.com>
On 12/11/20 4:41 AM, Yonghong Song wrote:
> Currently, when checking stack memory accessed by helper calls,
> for spills, only PTR_TO_BTF_ID and SCALAR_VALUE are
> allowed.
>
> Song discovered an issue where the below bpf program
> int dump_task(struct bpf_iter__task *ctx)
> {
> struct seq_file *seq = ctx->meta->seq;
> static char[] info = "abc";
> BPF_SEQ_PRINTF(seq, "%s\n", info);
> return 0;
> }
> may cause a verifier failure.
>
> The verifier output looks like:
> ; struct seq_file *seq = ctx->meta->seq;
> 1: (79) r1 = *(u64 *)(r1 +0)
> ; BPF_SEQ_PRINTF(seq, "%s\n", info);
> 2: (18) r2 = 0xffff9054400f6000
> 4: (7b) *(u64 *)(r10 -8) = r2
> 5: (bf) r4 = r10
> ;
> 6: (07) r4 += -8
> ; BPF_SEQ_PRINTF(seq, "%s\n", info);
> 7: (18) r2 = 0xffff9054400fe000
> 9: (b4) w3 = 4
> 10: (b4) w5 = 8
> 11: (85) call bpf_seq_printf#126
> R1_w=ptr_seq_file(id=0,off=0,imm=0) R2_w=map_value(id=0,off=0,ks=4,vs=4,imm=0)
> R3_w=inv4 R4_w=fp-8 R5_w=inv8 R10=fp0 fp-8_w=map_value
> last_idx 11 first_idx 0
> regs=8 stack=0 before 10: (b4) w5 = 8
> regs=8 stack=0 before 9: (b4) w3 = 4
> invalid indirect read from stack off -8+0 size 8
>
> Basically, the verifier complains the map_value pointer at "fp-8" location.
> To fix the issue, if env->allow_ptr_leaks is true, let us also permit
> pointers on the stack to be accessible by the helper.
>
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Reported-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> kernel/bpf/verifier.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 93def76cf32b..eebb2d3e16bf 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3769,7 +3769,9 @@ static int check_stack_boundary(struct bpf_verifier_env *env, int regno,
> goto mark;
>
> if (state->stack[spi].slot_type[0] == STACK_SPILL &&
> - state->stack[spi].spilled_ptr.type == SCALAR_VALUE) {
> + (state->stack[spi].spilled_ptr.type == SCALAR_VALUE ||
> + (state->stack[spi].spilled_ptr.type != NOT_INIT &&
Thinking more on this, your v2 was actually correct since in such case stype
would have been STACK_MISC or STACK_ZERO and we would have jumped to goto mark
here instead, so the above is not reachable under NOT_INIT. Anyway, I took the
v2 in, thanks!
> + env->allow_ptr_leaks))) {
> __mark_reg_unknown(env, &state->stack[spi].spilled_ptr);
> for (j = 0; j < BPF_REG_SIZE; j++)
> state->stack[spi].slot_type[j] = STACK_MISC;
>
next prev parent reply other threads:[~2020-12-14 21:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-11 3:41 [PATCH bpf-next v3 0/2] bpf: permits pointers on stack for helper calls Yonghong Song
2020-12-11 3:41 ` [PATCH bpf-next v3 1/2] " Yonghong Song
2020-12-14 21:26 ` Daniel Borkmann [this message]
2020-12-15 3:57 ` Yonghong Song
2020-12-11 3:41 ` [PATCH bpf-next v3 2/2] selftests/bpf: add a test for ptr_to_map_value on stack for helper access Yonghong Song
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=e48545be-6b03-aa2b-d5f6-a12b180ba116@iogearbox.net \
--to=daniel@iogearbox.net \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=kernel-team@fb.com \
--cc=songliubraving@fb.com \
--cc=yhs@fb.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.