All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Borkmann <daniel@iogearbox.net>
To: Tycho Andersen <tycho.andersen@canonical.com>,
	Alexei Starovoitov <ast@kernel.org>,
	"David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH] ebpf: emit correct src_reg for conditional jumps
Date: Fri, 11 Sep 2015 10:45:29 +0200	[thread overview]
Message-ID: <55F294A9.9020005@iogearbox.net> (raw)
In-Reply-To: <1441931107-17673-1-git-send-email-tycho.andersen@canonical.com>

On 09/11/2015 02:25 AM, Tycho Andersen wrote:
> Instead of always emitting BPF_REG_X, let's emit BPF_REG_X only when the
> source actually is BPF_X. This causes programs generated by the classic
> converter to not be importable via bpf(), as the eBPF verifier checks that
> the src_reg is correct or 0. While not a problem yet, this will be a
> problem when BPF_PROG_DUMP lands, and we can potentially dump and re-import
> programs generated by the converter.
>
> Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
> CC: Alexei Starovoitov <ast@kernel.org>
> CC: Daniel Borkmann <daniel@iogearbox.net>

I think the description at the beginning could have been a bit clearer. I.e.
it's safe to zero insn->src_reg for BPF_SRC(fp->code) that is not X, because
we can either have X or K for classic jump compares, and in case of K we're
only interested in the immediate value, but not a possible src_reg, of course,
so eBPF converter should have zeroed it.

Yeah, it was never really the aim of the converter to cover something like
that requirement you seem to have:

   load classic BPF
     -> transform into eBPF
       -> dump that eBPF result to uspace
         -> load that eBPF via bpf(2)

Anyway, it doesn't cause an issue in the current code as stated, but it's good
if we fix it up nevertheless.

Looks good to me:

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

  reply	other threads:[~2015-09-11  8:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-11  0:25 [PATCH] ebpf: emit correct src_reg for conditional jumps Tycho Andersen
2015-09-11  8:45 ` Daniel Borkmann [this message]
2015-09-11  9:28   ` Daniel Borkmann
2015-09-11 15:40     ` Alexei Starovoitov
2015-09-11 15:50       ` Tycho Andersen
2015-09-11 16:53         ` Daniel Borkmann
2015-09-11 21:53 ` David Miller

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=55F294A9.9020005@iogearbox.net \
    --to=daniel@iogearbox.net \
    --cc=ast@kernel.org \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=tycho.andersen@canonical.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.