RE: [PATCH 3/3] bpf: make add/subtract one program validate

[Konstantin Ananyev <konstantin.ananyev@huawei.com>]

> Add tests loading simple BPF programs adding or subtracting one to its
> argument and fix triggered signed integer overflow undefined behaviours:
> 
>     lib/bpf/bpf_validate.c:324:24: runtime error: signed integer
>     overflow: 1 + 9223372036854775807 cannot be represented in type
>     'long int'
> 
>     lib/bpf/bpf_validate.c:352:24: runtime error: signed integer
>     overflow: -9223372036854775808 - 1 cannot be represented in type
>     'long int'
> 
> As a minimal possible fix perform operation on unsigned integers where
> overflow is well-defined, which was probably the original intent.
> 
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> ---
>  app/test/test_bpf_simple.c | 58 ++++++++++++++++++++++++++++++++++++++
>  lib/bpf/bpf_validate.c     |  8 +++---
>  2 files changed, 62 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test/test_bpf_simple.c b/app/test/test_bpf_simple.c
> index 576a6ed029..d4c5bbdc6e 100644
> --- a/app/test/test_bpf_simple.c
> +++ b/app/test/test_bpf_simple.c
> @@ -129,3 +129,61 @@ test_simple_minimal_working(void)
> 
>  REGISTER_FAST_TEST(bpf_simple_minimal_working_autotest, true, true,
>  	test_simple_minimal_working);
> +
> +/*
> + * Try and load valid BPF program adding one to the argument.
> + */
> +static int
> +test_simple_add_one(void)
> +{
> +	static const struct ebpf_insn ins[] = {
> +		{
> +			/* Set return value to one. */
> +			.code = (EBPF_ALU64 | EBPF_MOV | BPF_K),
> +			.dst_reg = EBPF_REG_0,
> +			.imm = 1,
> +		},
> +		{
> +			/* Add program argument to the return value. */
> +			.code = (EBPF_ALU64 | BPF_ADD | BPF_X),
> +			.src_reg = EBPF_REG_1,
> +			.dst_reg = EBPF_REG_0,
> +		},
> +		{
> +			.code = (BPF_JMP | EBPF_EXIT),
> +		},
> +	};
> +	return simple_bpf_load_test(RTE_DIM(ins), ins, 0);
> +}
> +
> +REGISTER_FAST_TEST(bpf_simple_add_one_autotest, true, true,
> +	test_simple_add_one);
> +
> +/*
> + * Try and load valid BPF program subtracting one from the argument.
> + */
> +static int
> +test_simple_subtract_one(void)
> +{
> +	static const struct ebpf_insn ins[] = {
> +		{
> +			/* Subtract one from the program argument. */
> +			.code = (EBPF_ALU64 | BPF_SUB | BPF_K),
> +			.dst_reg = EBPF_REG_1,
> +			.imm = 1,
> +		},
> +		{
> +			/* Set return value to the result. */
> +			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
> +			.src_reg = EBPF_REG_1,
> +			.dst_reg = EBPF_REG_0,
> +		},
> +		{
> +			.code = (BPF_JMP | EBPF_EXIT),
> +		},
> +	};
> +	return simple_bpf_load_test(RTE_DIM(ins), ins, 0);
> +}
> +
> +REGISTER_FAST_TEST(bpf_simple_subtract_one_autotest, true, true,
> +	test_simple_subtract_one);
> diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
> index 23444b3eaa..47ad6fef0f 100644
> --- a/lib/bpf/bpf_validate.c
> +++ b/lib/bpf/bpf_validate.c
> @@ -243,8 +243,8 @@ eval_add(struct bpf_reg_val *rd, const struct bpf_reg_val
> *rs, uint64_t msk)
> 
>  	rv.u.min = (rd->u.min + rs->u.min) & msk;
>  	rv.u.max = (rd->u.max + rs->u.max) & msk;
> -	rv.s.min = (rd->s.min + rs->s.min) & msk;
> -	rv.s.max = (rd->s.max + rs->s.max) & msk;
> +	rv.s.min = ((uint64_t)rd->s.min + (uint64_t)rs->s.min) & msk;
> +	rv.s.max = ((uint64_t)rd->s.max + (uint64_t)rs->s.max) & msk;
> 
>  	/*
>  	 * if at least one of the operands is not constant,
> @@ -272,8 +272,8 @@ eval_sub(struct bpf_reg_val *rd, const struct bpf_reg_val
> *rs, uint64_t msk)
> 
>  	rv.u.min = (rd->u.min - rs->u.max) & msk;
>  	rv.u.max = (rd->u.max - rs->u.min) & msk;
> -	rv.s.min = (rd->s.min - rs->s.max) & msk;
> -	rv.s.max = (rd->s.max - rs->s.min) & msk;
> +	rv.s.min = ((uint64_t)rd->s.min - (uint64_t)rs->s.max) & msk;
> +	rv.s.max = ((uint64_t)rd->s.max - (uint64_t)rs->s.min) & msk;
> 
>  	/*
>  	 * if at least one of the operands is not constant,
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>

> 2.43.0