iovisor / ubpf

Userspace eBPF VM
Apache License 2.0
829 stars 136 forks source link

Fix atomic alu with fetch #546

Closed Alan-Jowett closed 1 month ago

Alan-Jowett commented 2 months ago

This pull request introduces support for atomic operations in the disassembler and improves the handling of atomic compare-exchange instructions in the JIT compiler. The most important changes include handling atomic operations in the disassembler, saving the original value of the source register in the JIT compiler, and ensuring atomic compare-exchange instructions are correctly implemented.

Disassembler Enhancements:

JIT Compiler Improvements:

coveralls commented 2 months ago

Coverage Status

coverage: 80.718% (-0.09%) from 80.809% when pulling c69fade777177e677086cbb36c0efe6e97d30ede on Alan-Jowett:fix_atomic_alu_with_fetch into 389cf54374d33b6a2b8940caeba3c9c212fb1e00 on iovisor:main.

Alan-Jowett commented 1 month ago

Additional failure:

alanjo@alanjo-dev2:~/ubpf$ libfuzzer/split.sh artifacts/crash-7d9bcc963763cd1f80da6c09dedea38e1435c4e5
Extracting program-7d9bcc963763cd1f80da6c09dedea38e1435c4e5...
Extracting memory-7d9bcc963763cd1f80da6c09dedea38e1435c4e5...
Disassembling program-7d9bcc963763cd1f80da6c09dedea38e1435c4e5...
Program size: 32
Memory size: 9
Disassembled program:
mov %r0, 0x2900
sub32 %r0, 0x40309c74
lock xchg [%r1 + 0x0], %r0
exit
Memory contents:
00000000: 0000 0000 0001 1900 00                   .........
interpreter_result: bfcf8c8c
jit_result: 19010000000000
Alan-Jowett commented 1 month ago
alanjo@alanjo-dev2:~/ubpf$ xxd  artifacts/program-7d9bcc963763cd1f80da6c09dedea38e1435c4e5
00000000: b700 0000 0029 0000 1400 7800 749c 3040  .....)....x.t.0@
00000010: db01 0000 e600 ff1a 9500 fdff 5bb1 0001  ............[...

Invalid instruction opcode: db + imm == e6.

This translates as EBPF_ATOMIC_OP_XCHG without the fetch bit set...

hawkinsw commented 1 month ago

I am sorry for the delay! I have started to review but did not get as far as I would have liked! I will (I hope) finish up tomorrow!

hawkinsw commented 1 month ago

On a related note, what do you think about "sprucing up" the disassembler's output with some optional debugging and warning information? Given the program in artifacts/program-7d9bcc963763cd1f80da6c09dedea38e1435c4e5, you would see something like

$ source file.asm  | ./bin/ubpf-disassembler 
mov %r0, 0x2900
Details:
    Class: 0x7
    Regs: 0x0
    Offset: 0x0
    Immediate: 0x2900
    Warnings: 
-----------------
sub32 %r0, 0x40309c74
Details:
    Class: 0x4
    Regs: 0x0
    Offset: 0x78
    Immediate: 0x40309c74
    Warnings: off has a value but it is not used by the instruction.
-----------------
stxdw [%r1], %r0
Details:
    Class: 0x3
    Regs: 0x1
    Offset: 0x0
    Immediate: 0x1aff00e6
    Warnings: imm has a value but it is not used by the instruction.
-----------------
exit
Details:
    Class: 0x5
    Regs: 0x0
    Offset: 0xfffd
    Immediate: 0x100b15b
    Warnings: off has a value but it is not used by the instruction; imm has a value but it is not used by the instruction.