Netdev List
 help / color / mirror / Atom feed
From: Josef Bacik <josef@toxicpanda.com>
To: Alexei Starovoitov <ast@fb.com>
Cc: Edward Cree <ecree@solarflare.com>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	netdev <netdev@vger.kernel.org>,
	David Miller <davem@davemloft.net>,
	iovisor-dev <iovisor-dev@lists.iovisor.org>,
	Josef Bacik <jbacik@fb.com>
Subject: Re: More BPF verifier questions
Date: Mon, 5 Jun 2017 18:47:55 +0000	[thread overview]
Message-ID: <20170605184754.GA30973@li70-116.members.linode.com> (raw)
In-Reply-To: <f75a8721-5cb0-f0bd-6224-b11bab1f036e@fb.com>

On Mon, Jun 05, 2017 at 11:11:05AM -0700, Alexei Starovoitov wrote:
> On 6/2/17 7:42 AM, Edward Cree wrote:
> >Also, I feel I haven't fully understood the semantics of {min,max}_value and
> > signed vs. unsigned comparisons.  It seems that currently reg_set_min_max
> > [_inv] assumes that any given register-value will either only be used as
> > signed, or only be used as unsigned — which while potentially reasonable
> > for compiler-generated bytecode, could easily be untrue of a hand-crafted
> > BPF program.
> >For instance, take BPF_JGT(reg, val).  This currently sets
> > false_reg->min_value to zero, but if val >= (1<<63), the false branch could
> > be taken for a value that's negative (when interpreted as signed).
> 
> I think the way Josef intended it to behave is min/max_value are
> absolute values that 64-bits can hold.
> In that sense unsigned (JGT) comparison and the false branch are
> implying that min_value = 0.
> but if we don't treat min/max consistently as sign-free numbers
> than indeed it can cause issues.
> Do you have an asm test case that demonstrates that?
>

Well the min_value is a s64, but yeah anything negative is supposed to be
rejected, so it essentially acts as the range of unsigned absolute values it can
hold.  I tried to hand craft a way to exploit this but I don't think it's
possible.  In the normal BPF_JGT path with your case we'd end up with

false_reg->min_value = 0;
false_reg->max_value = 1<<63 = BPF_REGISTER_MAX_RANGE
true_reg->min_value = BPF_REGISTER_MIN_RANGE

>From here we want to exploit the fact that false_reg->min_value is not
necessarily correct, but in order to do that we need to get false_reg->max_value
below the actual size limit for the data we're reaching into, which means we
want to _only_ change false_reg->max_value.  Thankfully there doesn't appear to
be a way to do that, everything changes either only min_value or both min_value
and max_value.  I think we're safe here, unless I've missed something.  Thanks,

Josef

  reply	other threads:[~2017-06-05 18:47 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-02 14:42 More BPF verifier questions Edward Cree
     [not found] ` <24a519bc-d403-f429-5d72-2a1d31edfbe7-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
2017-06-05  7:06   ` Y Song via iovisor-dev
2017-06-06 18:48     ` Edward Cree
2017-06-05 18:11 ` Alexei Starovoitov
2017-06-05 18:47   ` Josef Bacik [this message]
2017-06-06 14:17     ` Edward Cree

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=20170605184754.GA30973@li70-116.members.linode.com \
    --to=josef@toxicpanda.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ast@fb.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=ecree@solarflare.com \
    --cc=iovisor-dev@lists.iovisor.org \
    --cc=jbacik@fb.com \
    --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