Commit bb01a1bb authored by Daniel Borkmann's avatar Daniel Borkmann
Browse files

bpf: Fix mask direction swap upon off reg sign change 



Masking direction as indicated via mask_to_left is considered to be
calculated once and then used to derive pointer limits. Thus, this
needs to be placed into bpf_sanitize_info instead so we can pass it
to sanitize_ptr_alu() call after the pointer move. Piotr noticed a
corner case where the off reg causes masking direction change which
then results in an incorrect final aux->alu_limit.

Fixes: 7fedb63a ("bpf: Tighten speculative pointer arithmetic mask")
Reported-by: default avatarPiotr Krysiuk <piotras@gmail.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Reviewed-by: default avatarPiotr Krysiuk <piotras@gmail.com>
Acked-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent 3d0220f6

kernel/bpf/verifier.c

+12 −10
Original line number Diff line number Diff line
@@ -6409,18 +6409,10 @@ enum { 
};



static int retrieve_ptr_limit(const struct bpf_reg_state *ptr_reg,

			      const struct bpf_reg_state *off_reg,

			      u32 *alu_limit, u8 opcode)

			      u32 *alu_limit, bool mask_to_left)

{

	bool off_is_neg = off_reg->smin_value < 0;

	bool mask_to_left = (opcode == BPF_ADD &&  off_is_neg) ||

			    (opcode == BPF_SUB && !off_is_neg);

	u32 max = 0, ptr_limit = 0;



	if (!tnum_is_const(off_reg->var_off) &&

	    (off_reg->smin_value < 0) != (off_reg->smax_value < 0))

		return REASON_BOUNDS;



	switch (ptr_reg->type) {

	case PTR_TO_STACK:

		/* Offset 0 is out-of-bounds, but acceptable start for the
@@ -6488,6 +6480,7 @@ static bool sanitize_needed(u8 opcode) 


struct bpf_sanitize_info {

	struct bpf_insn_aux_data aux;

	bool mask_to_left;

};



static int sanitize_ptr_alu(struct bpf_verifier_env *env,
@@ -6519,7 +6512,16 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env, 
	if (vstate->speculative)

		goto do_sim;



	err = retrieve_ptr_limit(ptr_reg, off_reg, &alu_limit, opcode);

	if (!commit_window) {

		if (!tnum_is_const(off_reg->var_off) &&

		    (off_reg->smin_value < 0) != (off_reg->smax_value < 0))

			return REASON_BOUNDS;



		info->mask_to_left = (opcode == BPF_ADD &&  off_is_neg) ||

				     (opcode == BPF_SUB && !off_is_neg);

	}



	err = retrieve_ptr_limit(ptr_reg, &alu_limit, info->mask_to_left);

	if (err < 0)

		return err;