diff mbox series

[v8,03/12] bpf: allow self-xor operation

Message ID	20210913181510.46058-4-stephen@networkplumber.org (mailing list archive)
State	Superseded, archived
Delegated to:	Thomas Monjalon
Headers	From: Stephen Hemminger <stephen@networkplumber.org> To: dev@dpdk.org Cc: Stephen Hemminger <stephen@networkplumber.org> Date: Mon, 13 Sep 2021 11:15:01 -0700 Message-Id: <20210913181510.46058-4-stephen@networkplumber.org> In-Reply-To: <20210913181510.46058-1-stephen@networkplumber.org> References: <20210903004732.109023-1-stephen@networkplumber.org> <20210913181510.46058-1-stephen@networkplumber.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH v8 03/12] bpf: allow self-xor operation Precedence: list Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org>
Series	Packet capture framework enhancements \| [v8,00/12] Packet capture framework enhancements [v8,01/12] librte_pcapng: add new library for writing pcapng files [v8,02/12] lib: pdump is not supported on Windows [v8,03/12] bpf: allow self-xor operation [v8,04/12] bpf: add function to convert classic BPF to DPDK BPF [v8,05/12] bpf: add function to dump eBPF instructions [v8,06/12] pdump: support pcapng and filtering [v8,07/12] app/dumpcap: add new packet capture application [v8,08/12] test: add test for bpf_convert [v8,09/12] test: add a test for pcapng library [v8,10/12] test: enable bpf autotest [v8,11/12] doc: changes for new pcapng and dumpcap [v8,12/12] MAINTAINERS: add entry for new packet capture features

Checks

Context	Check	Description
ci/checkpatch	success	coding style OK

Commit Message

Stephen Hemminger Sept. 13, 2021, 6:15 p.m. UTC

  When doing BPF filter program conversion, a common way
to zero a register in single instruction is:
     xor r7,r7
The BPF validator would not allow this because the value of
r7 was undefined. But after this operation it always zero.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/bpf/bpf_validate.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Ananyev, Konstantin Sept. 15, 2021, 10:55 a.m. UTC | #1

> When doing BPF filter program conversion, a common way
> to zero a register in single instruction is:
>      xor r7,r7
> The BPF validator would not allow this because the value of
> r7 was undefined. But after this operation it always zero.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/bpf/bpf_validate.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
> index 7b1291b382e9..7647a7454dc2 100644
> --- a/lib/bpf/bpf_validate.c
> +++ b/lib/bpf/bpf_validate.c
> @@ -661,8 +661,12 @@ eval_alu(struct bpf_verifier *bvf, const struct ebpf_insn *ins)
> 
>  	op = BPF_OP(ins->code);
> 
> -	err = eval_defined((op != EBPF_MOV) ? rd : NULL,
> -			(op != BPF_NEG) ? &rs : NULL);
> +	/* Allow self-xor as way to zero register */
> +	if (op == BPF_XOR && ins->src_reg == ins->dst_reg)
> +		err = NULL;
> +	else
> +		err = eval_defined((op != EBPF_MOV) ? rd : NULL,
> +				   (op != BPF_NEG) ? &rs : NULL);

Two things:
- We probably need to check that this is instruction with source register (not imm value).
- rd value is not evaluated to zero, while it probably should
  (will help evaluator to better predict further values) 

So might be better to do something like:

/* Allow self-xor as way to zero register */
        if (op == BPF_XOR && BPF_SRC(ins->code) == BPF_X &&
                        ins->src_reg == ins->dst_reg) {
                eval_fill_imm(&rs, UINT64_MAX, 0);
                eval_fill_imm(rd, UINT64_MAX, 0);
        }

        err = eval_defined((op != EBPF_MOV) ? rd : NULL,
                           (op != BPF_NEG) ? &rs : NULL);
        if (err != NULL)
                return err;

...

Another thing - shouldn't that patch be treated like a fix (cc to stable, etc.)?

>  	if (err != NULL)
>  		return err;
> 
> --
> 2.30.2

diff mbox series

Patch

diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 7b1291b382e9..7647a7454dc2 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -661,8 +661,12 @@  eval_alu(struct bpf_verifier *bvf, const struct ebpf_insn *ins)
 
 	op = BPF_OP(ins->code);
 
-	err = eval_defined((op != EBPF_MOV) ? rd : NULL,
-			(op != BPF_NEG) ? &rs : NULL);
+	/* Allow self-xor as way to zero register */
+	if (op == BPF_XOR && ins->src_reg == ins->dst_reg)
+		err = NULL;
+	else
+		err = eval_defined((op != EBPF_MOV) ? rd : NULL,
+				   (op != BPF_NEG) ? &rs : NULL);
 	if (err != NULL)
 		return err;