Re: [PATCH bpf-next 1/3] bpf: allow ctx writes using BPF_ST_MEM instruction

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2023-03-03 at 12:21 -0800, Alexei Starovoitov wrote:
> On Fri, Mar 03, 2023 at 12:55:05AM +0200, Eduard Zingerman wrote:
> > -			prev_src_type = &env->insn_aux_data[env->insn_idx].ptr_type;
> > -
> > -			if (*prev_src_type == NOT_INIT) {
> > -				/* saw a valid insn
> > -				 * dst_reg = *(u32 *)(src_reg + off)
> > -				 * save type to validate intersecting paths
> > -				 */
> > -				*prev_src_type = src_reg_type;
> > -
> > -			} else if (reg_type_mismatch(src_reg_type, *prev_src_type)) {
> > -				/* ABuser program is trying to use the same insn
> > -				 * dst_reg = *(u32*) (src_reg + off)
> > -				 * with different pointer types:
> > -				 * src_reg == ctx in one branch and
> > -				 * src_reg == stack|map in some other branch.
> > -				 * Reject it.
> > -				 */
> > -				verbose(env, "same insn cannot be used with different pointers\n");
> > -				return -EINVAL;
> 
> There is a merge conflict with this part.
> LDX is now handled slightly differently comparing to STX.

I changed save_aux_ptr_type() as below:

  static int save_aux_ptr_type(struct bpf_verifier_env *env, enum bpf_reg_type type,
  			     bool allow_trust_missmatch)
  {
  	enum bpf_reg_type *prev_type = &env->insn_aux_data[env->insn_idx].ptr_type;
    ...
  	if (*prev_type == NOT_INIT) {
        ...
  	} else if (reg_type_mismatch(type, *prev_type)) {
  		/* Abuser program is trying to use the same insn
         * ...
  		 */
  		if (allow_trust_missmatch &&
  		    base_type(type) == PTR_TO_BTF_ID &&
  		    base_type(*prev_type) == PTR_TO_BTF_ID) {
  			/*
  			 * Have to support a use case when one path through
  			 * the program yields TRUSTED pointer while another
  			 * is UNTRUSTED. Fallback to UNTRUSTED to generate
  			 * BPF_PROBE_MEM.
  			 */
  			*prev_type = PTR_TO_BTF_ID | PTR_UNTRUSTED;
  		} else {
  			verbose(env, "same insn cannot be used with different pointers\n");
  			return -EINVAL;
  		}
  	}
  
  	return 0;
  }
  
But I don't understand why is it allowed to dereference untrusted
pointers for LDX but not for ST/STX.
  
[...]