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
next prev parent 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