On 02/23/2017 11:00 PM, William Tu wrote:
Hi Daniel,

Thanks for the reply! I understand the workaround, can you suggest a
way to try it? Should I compile the C source into llvm IR bitcode, add
thie "r0 &= 0xffff", then assembly it to bpf target binary.

Hmm, problem with C code is that we might have no control over when llvm
decides to spill to stack where this workaround would then be needed. Your
suggestion could work, maybe an llvm expert can jump in here if we have
some other, easier options at hand?

Second case is to preserve the type in verifier when spilled, but will make
pruning ineffective to the point that we could see regressions of programs
not loading anymore (due to running over BPF_COMPLEXITY_LIMIT_INSNS).
states_equal() wrt probing stack space doesn't have much optimizations
thus far either; certainly this path would need a careful evaluation.

On Wed, Feb 22, 2017 at 9:08 AM, Daniel Borkmann <dan...@iogearbox.net> wrote:
On 02/17/2017 06:07 PM, William Tu via iovisor-dev wrote:

Hi,

I encountered another BPF verifier issue related to my previous one
https://lists.iovisor.org/pipermail/iovisor-dev/2017-January/000603.html

My guess is that when register spills to stack and restore, the state
of imm upper zero bits does not get restore? Any comments are
appreciated!

This time the verifier shows:
---
   R0=imm0,min_value=0,max_value=0 R1=imm58,min_value=58,max_value=58
R3=pkt(id=0,off=58,r=58) R4=inv61 R5=pkt_end
R6=imm144,min_value=144,max_value=144 R7=imm0,min_value=0,max_value=0
R8=ctx R9=pkt(id=0,off=0,r=58) R10=fp
260: (bf) r5 = r6
261: (47) r5 |= 12
262: (bf) r1 = r5
263: (07) r1 += 44
264: (77) r1 >>= 3
265: (7b) *(u64 *)(r10 -64) = r1
266: (bf) r7 = r5
267: (07) r7 += 36
268: (77) r7 >>= 3
269: (bf) r0 = r5
270: (07) r0 += 20
271: (77) r0 >>= 3
272: (bf) r1 = r5
273: (07) r1 += 52
274: (77) r1 >>= 3
275: (77) r6 >>= 3
276: (79) r2 = *(u64 *)(r10 -24)
277: (bf) r2 = r5
278: (77) r2 >>= 3
279: (7b) *(u64 *)(r10 -184) = r2
280: (07) r5 += 180
281: (77) r5 >>= 3
282: (bf) r4 = r9
283: (0f) r4 += r5
284: (47) r5 |= 1
285: (bf) r3 = r9
286: (0f) r3 += r6
287: (bf) r6 = r9
288: (0f) r6 += r1
289: (47) r1 |= 1
290: (bf) r2 = r9
291: (0f) r2 += r0
292: (7b) *(u64 *)(r10 -312) = r2
293: (bf) r2 = r9
294: (79) r0 = *(u64 *)(r10 -184)
295: (0f) r2 += r0
cannot add integer value with 0 upper zero bits to ptr_to_packet

Right, so lines 260, 261 r5 is imm type, which in 277 is transferred
to r2 and eventually in 279 spilled to stack. check_stack_write() would
see that type is not spillable, so it will be stored as regular data
to stack (STACK_MISC). Later read in 294 will restore that to r0.
check_stack_read() sees that stack was marked as STACK_MISC and marks
reg as unknown, where you'll later get the error when added to ptr_to_packet
as no overflow protection can be guaranteed (imm is 0). Afaik (Alexei,
might correct me if I'm wrong), if we would support spilling imm, this
could have a non-trivial increase in complexity, where verifier would
need to do a lot more work due to pruning not taking effect. If you
look at evaluate_reg_alu(), you could try to work around it for the time
being by doing r0 &= 0xffff right before the r2 += r0.

Thanks,
Daniel
_______________________________________________
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev

Reply via email to