All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: John Fastabend <john.fastabend@gmail.com>
Cc: Yonghong Song <yhs@fb.com>, bpf <bpf@vger.kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kernel Team <kernel-team@fb.com>,
	Edward Cree <ecree@solarflare.com>
Subject: Re: [PATCH bpf-next 1/2] bpf: fix a verifier failure with xor
Date: Wed, 26 Aug 2020 22:12:42 -0700	[thread overview]
Message-ID: <CAADnVQ+XYd=GzF2P=3RO_Xi6m5zQA2q3JYTWxbh3O=Pfn8zLXw@mail.gmail.com> (raw)
In-Reply-To: <5f46dcd8c0156_50e8208f4@john-XPS-13-9370.notmuch>

On Wed, Aug 26, 2020 at 3:06 PM John Fastabend <john.fastabend@gmail.com> wrote:
>
> It is a hold-out from when we went from having a 32-bit var-off
> and a 64-bit var-off. I'll send a patch its clumsy and not needed
> for sure.

please follow up with such patches.

> The other subtle piece here we should clean up. Its possible
> to have a const in the subreg but a non-const in the wider
> 64-bit reg. In this case we skip marking the 32-bit subreg
> as known and rely on the 64-bit case to handle it. But, we
> may if the 64-bit reg is not const fall through and update
> the 64-bit bounds. Then later we call __update_reg32_bounds()
> and this will use the var_off, previously updated. The
> 32-bit bounds are then updated using this var_off so they
> are correct even if less precise than we might expect. I
> believe xor is correct here as well.

makes sense. I think it's correct now, but I agree that cleaning
this up would be good as well.

> I need to send another patch with a comment for the BTF_ID
> types. I'll add some test cases for this 64-bit non-const and
> subreg const so we don't break it later. I'm on the fence
> if we should tighten the bounds there as well. I'll see if
> it helps readability to do explicit 32-bit const handling
> there. I had it in one of the early series with the 32-bit
> bounds handling, but dropped for what we have now.

Not following. Why is this related to btf_id ?

> LGTM, but I see a couple follow up patches with tests, comments,
> and dropping the duplicate ALU op I'll try to do those Friday, unless
> someone else does them first.

yes. please :)

I've pushed this set to bpf-next in the meantime.
Thanks everyone!

  reply	other threads:[~2020-08-27  5:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-25  6:46 [PATCH bpf-next 0/2] fix a verifier failure with xor Yonghong Song
2020-08-25  6:46 ` [PATCH bpf-next 1/2] bpf: " Yonghong Song
2020-08-26  1:58   ` Alexei Starovoitov
2020-08-26  3:36     ` Yonghong Song
2020-08-26 22:06       ` John Fastabend
2020-08-27  5:12         ` Alexei Starovoitov [this message]
2020-08-27 18:43           ` John Fastabend
2020-09-01 20:07   ` Andrii Nakryiko
2020-09-02  2:17     ` Yonghong Song
2020-09-02  5:27       ` John Fastabend
2020-09-02  5:43         ` Yonghong Song
2020-09-04  5:29           ` John Fastabend
2020-09-02  9:33       ` Toke Høiland-Jørgensen
2020-09-02 14:21         ` Alexei Starovoitov
2020-09-02 15:01           ` Toke Høiland-Jørgensen
2020-09-02 21:40             ` Alexei Starovoitov
2020-08-25  6:46 ` [PATCH bpf-next 2/2] selftests/bpf: add verifier tests for xor operation 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='CAADnVQ+XYd=GzF2P=3RO_Xi6m5zQA2q3JYTWxbh3O=Pfn8zLXw@mail.gmail.com' \
    --to=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=ecree@solarflare.com \
    --cc=john.fastabend@gmail.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 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.