bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: Edward Cree <ecree@solarflare.com>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	John Fastabend <john.fastabend@gmail.com>
Cc: yhs@fb.com, daniel@iogearbox.net, netdev@vger.kernel.org,
	bpf@vger.kernel.org
Subject: Re: [RFC PATCH 2/4] bpf: verifier, do explicit u32 bounds tracking
Date: Tue, 10 Mar 2020 12:24:39 -0700	[thread overview]
Message-ID: <5e67e977eb4f_586d2b10f16785b8f5@john-XPS-13-9370.notmuch> (raw)
In-Reply-To: <3f80b587-c5b0-0446-8cbc-eff1758496e9@solarflare.com>

Edward Cree wrote:
> On 09/03/2020 23:58, Alexei Starovoitov wrote:> Thinking about it differently... var_off is a bit representation of
> > 64-bit register. So that bit representation doesn't really have
> > 32 or 16-bit chunks. It's a full 64-bit register. I think all alu32
> > and jmp32 ops can update var_off without losing information.
> Agreed; AFAICT the 32-bit var_off should always just be the bottom
>  32 bits of the full var_off.

This seems to work.


> In fact, it seems like the only situations where 32-bit bounds are
>  needed are (a) the high and low halves of a 64-bit register are
>  being used separately, so e.g. r0 = (x << 32) | y with small known
>  bounds on y you want to track, or (b) 32-bit signed arithmetic.
> (a) doesn't seem like it's in scope to be supported, and (b) should
>  (I'm naïvely imagining) only need the s32 bounds, not the u32 or
>  var32.

I guess I'm not opposed to supporting (a) it seems like it should
be doable.

For (b) the primary reason is to keep symmetry between 32-bit and
64-bit cases. But also we could have mixed signed 32-bit comparisons
which this helps with.

Example tracking bounds with [x,y] being signed 32-bit
bounds and [x',y'] being unsigned 32-bit bounds.

    r1 = #                   [x,y],[x',y']
    w1 >    0 goto pc+y      [x,y],[1 ,y']
    w1 s> -10 goto pc+x      [-10,y],[1 ,y']

We can't really deduce much from that in __reg_deduce_bounds so
we get stuck with different bounds on signed and unsigned space.
Same case as 64-bit world fwiw. I guess we could do more work
and use 64-bit/32-bit together and deduce something but I find
this more work than its worth. Keeping separate signed/unsigned
makes things easy.

> 
> John Fastabend wrote:
> > For example, BPF_ADD will do a tnum_add() this is a different
> > operation when overflows happen compared to tnum32_add(). Simply
> > truncating tnum_add result to 32-bits is not the same operation.
> I don't see why.  Overflows from the low (tracked) 32 bits can only
>  affect the high 32.
>  Truncation should be a homomorphism from
>  Z_2^n to Z_2^m wrt. both addition and multiplication, and tnums
>  are just (a particular class of) subsets of those rings.

Agreed, no need for 32bit ops on tnums.

> Can you construct an example of a tnum addition that breaks the
>  homomorphism?

no, I'm convinced. There seems to be a proof that the above is
true if n>m.

> 
> -ed



  reply	other threads:[~2020-03-10 19:24 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-07  0:10 [RFC bpf PATCH 0/4] rfc for 32-bit subreg verifier tracking John Fastabend
2020-03-07  0:10 ` [RFC PATCH 1/4] bpf: verifer, refactor adjust_scalar_min_max_vals John Fastabend
2020-03-07  0:11 ` [RFC PATCH 2/4] bpf: verifier, do explicit u32 bounds tracking John Fastabend
2020-03-07  0:22   ` John Fastabend
2020-03-09  5:39     ` Yonghong Song
2020-03-09 23:58   ` Alexei Starovoitov
2020-03-10 17:04     ` John Fastabend
2020-03-10 17:12     ` Edward Cree
2020-03-10 19:24       ` John Fastabend [this message]
2020-03-10 19:41         ` Edward Cree
2020-03-10 17:52   ` Yonghong Song
2020-03-10 19:54     ` John Fastabend
2020-03-07  0:11 ` [RFC PATCH 3/4] bpf: verifier, do_refine_retval_range may clamp umin to 0 incorrectly John Fastabend
2020-03-07  0:11 ` [RFC PATCH 4/4] bpf: selftests, bpf_get_stack return value add <0 John Fastabend

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=5e67e977eb4f_586d2b10f16785b8f5@john-XPS-13-9370.notmuch \
    --to=john.fastabend@gmail.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=ecree@solarflare.com \
    --cc=netdev@vger.kernel.org \
    --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).