On Sun, 2023-09-17 at 21:12 +0200, Greg Kroah-Hartman wrote: > 5.10-stable review patch. If anyone has any objections, please let me know. I believe Luis Gerhorst posted an objection to this patch for 6.1 in [1], for reasons described in [2]. The objection is relevant for 5.10 as well (does not depend on kernel version, actually). [1] https://lore.kernel.org/all/2023091653-peso-sprint-889d@gregkh/ [2] https://lore.kernel.org/bpf/20230913122827.91591-1-gerhorst@xxxxxxxxx/ > > ------------------ > > From: Yafang Shao <laoar.shao@xxxxxxxxx> > > commit d75e30dddf73449bc2d10bb8e2f1a2c446bc67a2 upstream. > > After we converted the capabilities of our networking-bpf program from > cap_sys_admin to cap_net_admin+cap_bpf, our networking-bpf program > failed to start. Because it failed the bpf verifier, and the error log > is "R3 pointer comparison prohibited". > > A simple reproducer as follows, > > SEC("cls-ingress") > int ingress(struct __sk_buff *skb) > { > struct iphdr *iph = (void *)(long)skb->data + sizeof(struct ethhdr); > > if ((long)(iph + 1) > (long)skb->data_end) > return TC_ACT_STOLEN; > return TC_ACT_OK; > } > > Per discussion with Yonghong and Alexei [1], comparison of two packet > pointers is not a pointer leak. This patch fixes it. > > Our local kernel is 6.1.y and we expect this fix to be backported to > 6.1.y, so stable is CCed. > > [1]. https://lore.kernel.org/bpf/CAADnVQ+Nmspr7Si+pxWn8zkE7hX-7s93ugwC+94aXSy4uQ9vBg@xxxxxxxxxxxxxx/ > > Suggested-by: Yonghong Song <yonghong.song@xxxxxxxxx> > Suggested-by: Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> > Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx> > Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > Link: https://lore.kernel.org/r/20230823020703.3790-2-laoar.shao@xxxxxxxxx > Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > --- > kernel/bpf/verifier.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -8178,6 +8178,12 @@ static int check_cond_jmp_op(struct bpf_ > return -EINVAL; > } > > + /* check src2 operand */ > + err = check_reg_arg(env, insn->dst_reg, SRC_OP); > + if (err) > + return err; > + > + dst_reg = ®s[insn->dst_reg]; > if (BPF_SRC(insn->code) == BPF_X) { > if (insn->imm != 0) { > verbose(env, "BPF_JMP/JMP32 uses reserved fields\n"); > @@ -8189,12 +8195,13 @@ static int check_cond_jmp_op(struct bpf_ > if (err) > return err; > > - if (is_pointer_value(env, insn->src_reg)) { > + src_reg = ®s[insn->src_reg]; > + if (!(reg_is_pkt_pointer_any(dst_reg) && reg_is_pkt_pointer_any(src_reg)) && > + is_pointer_value(env, insn->src_reg)) { > verbose(env, "R%d pointer comparison prohibited\n", > insn->src_reg); > return -EACCES; > } > - src_reg = ®s[insn->src_reg]; > } else { > if (insn->src_reg != BPF_REG_0) { > verbose(env, "BPF_JMP/JMP32 uses reserved fields\n"); > @@ -8202,12 +8209,6 @@ static int check_cond_jmp_op(struct bpf_ > } > } > > - /* check src2 operand */ > - err = check_reg_arg(env, insn->dst_reg, SRC_OP); > - if (err) > - return err; > - > - dst_reg = ®s[insn->dst_reg]; > is_jmp32 = BPF_CLASS(insn->code) == BPF_JMP32; > > if (BPF_SRC(insn->code) == BPF_K) { > >
