Re: [RFC bpf 1/2] bpf: Fix precision tracking for BPF_ALU | BPF_TO_BE | BPF_END

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2023-10-30 at 21:21 +0800, Shung-Hsi Yu wrote:
> BPF_END and BPF_NEG has a different specification for the source bit in
> the opcode compared to other ALU/ALU64 instructions, and is either
> reserved or use to specify the byte swap endianness. In both cases the
> source bit does not encode source operand location, and src_reg is a
> reserved field.
> 
> backtrack_insn() currently does not differentiate BPF_END and BPF_NEG
> from other ALU/ALU64 instructions, which leads to r0 being incorrectly
> marked as precise when processing BPF_ALU | BPF_TO_BE | BPF_END
> instructions. This commit teaches backtrack_insn() to correctly mark
> precision for such case.
> 
> While precise tracking of BPF_NEG and other BPF_END instructions are
> correct and does not need fixing because their source bit are unset and
> thus treated as the BPF_K case, this commit opt to process all BPF_NEG
> and BPF_END instructions within the same if-clause so it better aligns
> with current convention used in the verifier (e.g. check_alu_op).
> 
> Fixes: b5dc0163d8fd ("bpf: precise scalar_value tracking")
> Cc: stable@vger.kernel.org
> Reported-by: Mohamed Mahmoud <mmahmoud@redhat.com>
> Tested-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Tested-by: Tao Lyu <tao.lyu@epfl.ch>
> Signed-off-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

> ---
>  kernel/bpf/verifier.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 873ade146f3d..646dc49263fd 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3426,7 +3426,12 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>  	if (class == BPF_ALU || class == BPF_ALU64) {
>  		if (!bt_is_reg_set(bt, dreg))
>  			return 0;
> -		if (opcode == BPF_MOV) {
> +		if (opcode == BPF_END || opcode == BPF_NEG) {
> +			/* sreg is reserved and unused
> +			 * dreg still need precision before this insn
> +			 */
> +			return 0;
> +		} else if (opcode == BPF_MOV) {
>  			if (BPF_SRC(insn->code) == BPF_X) {
>  				/* dreg = sreg or dreg = (s8, s16, s32)sreg
>  				 * dreg needs precision after this insn