Skip to content

Commit e9bd9c4

Browse files
eddyz87borkmann
authored andcommitted
bpf: sync_linked_regs() must preserve subreg_def
Range propagation must not affect subreg_def marks, otherwise the following example is rewritten by verifier incorrectly when BPF_F_TEST_RND_HI32 flag is set: 0: call bpf_ktime_get_ns call bpf_ktime_get_ns 1: r0 &= 0x7fffffff after verifier r0 &= 0x7fffffff 2: w1 = w0 rewrites w1 = w0 3: if w0 < 10 goto +0 --------------> r11 = 0x2f5674a6 (r) 4: r1 >>= 32 r11 <<= 32 (r) 5: r0 = r1 r1 |= r11 (r) 6: exit; if w0 < 0xa goto pc+0 r1 >>= 32 r0 = r1 exit (or zero extension of w1 at (2) is missing for architectures that require zero extension for upper register half). The following happens w/o this patch: - r0 is marked as not a subreg at (0); - w1 is marked as subreg at (2); - w1 subreg_def is overridden at (3) by copy_register_state(); - w1 is read at (5) but mark_insn_zext() does not mark (2) for zero extension, because w1 subreg_def is not set; - because of BPF_F_TEST_RND_HI32 flag verifier inserts random value for hi32 bits of (2) (marked (r)); - this random value is read at (5). Fixes: 7574883 ("bpf: Propagate scalar ranges through register assignments.") Reported-by: Lonial Con <kongln9170@gmail.com> Signed-off-by: Lonial Con <kongln9170@gmail.com> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Acked-by: Daniel Borkmann <daniel@iogearbox.net> Closes: https://lore.kernel.org/bpf/7e2aa30a62d740db182c170fdd8f81c596df280d.camel@gmail.com Link: https://lore.kernel.org/bpf/20240924210844.1758441-1-eddyz87@gmail.com
1 parent 8b62645 commit e9bd9c4

File tree

1 file changed

+5
-0
lines changed

1 file changed

+5
-0
lines changed

kernel/bpf/verifier.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15326,8 +15326,12 @@ static void sync_linked_regs(struct bpf_verifier_state *vstate, struct bpf_reg_s
1532615326
continue;
1532715327
if ((!(reg->id & BPF_ADD_CONST) && !(known_reg->id & BPF_ADD_CONST)) ||
1532815328
reg->off == known_reg->off) {
15329+
s32 saved_subreg_def = reg->subreg_def;
15330+
1532915331
copy_register_state(reg, known_reg);
15332+
reg->subreg_def = saved_subreg_def;
1533015333
} else {
15334+
s32 saved_subreg_def = reg->subreg_def;
1533115335
s32 saved_off = reg->off;
1533215336

1533315337
fake_reg.type = SCALAR_VALUE;
@@ -15340,6 +15344,7 @@ static void sync_linked_regs(struct bpf_verifier_state *vstate, struct bpf_reg_s
1534015344
* otherwise another sync_linked_regs() will be incorrect.
1534115345
*/
1534215346
reg->off = saved_off;
15347+
reg->subreg_def = saved_subreg_def;
1534315348

1534415349
scalar32_min_max_add(reg, &fake_reg);
1534515350
scalar_min_max_add(reg, &fake_reg);

0 commit comments

Comments
 (0)