bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Martin KaFai Lau <kafai@fb.com>
Cc: bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	kernel-team@fb.com, Yonghong Song <yhs@fb.com>
Subject: Re: [PATCH bpf-next 3/4] bpf: selftest: A bpf prog that has a 32bit scalar spill
Date: Mon, 20 Sep 2021 19:32:34 -0700	[thread overview]
Message-ID: <20210921023234.pjnby3s4q4o4agwe@ast-mbp> (raw)
In-Reply-To: <20210921013122.1037548-1-kafai@fb.com>

On Mon, Sep 20, 2021 at 06:31:22PM -0700, Martin KaFai Lau wrote:
> It is a simplified example that can trigger a 32bit scalar spill.
> The const scalar is refilled and added to a skb->data later.
> Since the reg state of the 32bit scalar spill is not saved now,
> adding the refilled reg to skb->data and then comparing it with
> skb->data_end cannot verify the skb->data access.
> 
> With the earlier verifier patch and the llvm patch [1].  The verifier
> can correctly verify the bpf prog.

Let's land llvm patch and wait until CI picks up the new llvm build?
Please add a comment to selftests/bpf/README.rst that describes
the failing test when llvm is old.
I'm guessing there is no easier way to reliably skip the test
in such situation, since failure to load might be the result
of some future changes.
llvm version check won't work either.

the patch 2 looks correct to me. I couldn't spot any issue with the logic.

  reply	other threads:[~2021-09-21  3:25 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-21  1:31 [PATCH bpf-next 0/4] bpf: Support <8-byte scalar spill and refill Martin KaFai Lau
2021-09-21  1:31 ` [PATCH bpf-next 1/4] bpf: Check the other end of slot_type for STACK_SPILL Martin KaFai Lau
2021-09-21  1:31 ` [PATCH bpf-next 2/4] bpf: Support <8-byte scalar spill and refill Martin KaFai Lau
2021-09-21  1:31 ` [PATCH bpf-next 3/4] bpf: selftest: A bpf prog that has a 32bit scalar spill Martin KaFai Lau
2021-09-21  2:32   ` Alexei Starovoitov [this message]
2021-09-21 23:52     ` Martin KaFai Lau
2021-09-21  1:31 ` [PATCH bpf-next 4/4] bpf: selftest: Add verifier tests for <8-byte scalar spill and refill Martin KaFai Lau

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=20210921023234.pjnby3s4q4o4agwe@ast-mbp \
    --to=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kafai@fb.com \
    --cc=kernel-team@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 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).