Fix disassembly of eBPF atomic instructions and semantics of compare-and-exchange

[niooss-ledger]

Hello,

eBPF ISA v3 introduced atomic instructions: https://www.kernel.org/doc/html/v6.0/bpf/instruction-set.html#atomic-operations . These instructions are encoded using BPF_ATOMIC | BPF_W | BPF_STX and BPF_ATOMIC | BPF_DW | BPF_STX for 32-bit and 64-bit operations, with:

BPF_ATOMIC = 0xc0
BPF_DW = 0x18
BPF_W = 0
BPF_STX = 0x03

While Ghidra's semantic section is constructed correctly (atomic add uses an addition ; atomic or uses or ; ...), the disassembly always displays STXXADDW and STXXADDDW. These mnemonics come from the deprecated name BPF_XADD = BPF_ATOMIC | BPF_ADD = 0xc0.

This Pull Request replaces the confusing mnemonics with the ones used by binutils and documented in
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gas/doc/c-bpf.texi;h=003cb92a457985038a9abc1ffbf347f636eb0586;hb=2bc7af1ff7732451b6a7b09462a815c3284f9613#l745

While testing this, I found a bug in the semantics of atomic compare-and-exchange instruction, described in the next section.

Testing

I wrote a test C program, atomic.c using gcc's atomic built-ins. This program contains many small functions such as:

void atomic_exchange64(volatile unsigned long *ptr, unsigned long *val, unsigned long *ret) {
    __atomic_exchange(ptr, val, ret, __ATOMIC_RELAXED);
}

unsigned long atomic_fetch_add64(volatile unsigned long *addr, unsigned long value) {
    return __atomic_fetch_add(addr, value, __ATOMIC_RELAXED);
}

I compiled this program in a Debian 13 container with different options:

sudo apt-get install clang gcc-bpf llvm
clang -Wall -Wextra -O2 -target bpf -mcpu=v1 -c atomic.c -o compiled/atomic.deb13-clangO2-v1.ebpf
clang -Wall -Wextra -O2 -target bpf -mcpu=v4 -c atomic.c -o compiled/atomic.deb13-clangO2-v4.ebpf
bpf-gcc -Wall -Wextra -O2 -mcpu=v1 -c atomic.c -o compiled/atomic.deb13-gccO2-v1.ebpf
bpf-gcc -Wall -Wextra -O2 -mcpu=v4 -c atomic.c -o compiled/atomic.deb13-gccO2-v4.ebpf

I then disassembled the programs with LLVM's llvm-objdump and binutils' bpf-objdump:

llvm-objdump -rd compiled/atomic.deb13-clangO2-v1.ebpf > atomic.deb13-clangO2-v1.txt
llvm-objdump -rd compiled/atomic.deb13-clangO2-v4.ebpf > atomic.deb13-clangO2-v4.txt
bpf-objdump -rd compiled/atomic.deb13-gccO2-v1.ebpf > atomic.deb13-gccO2-v1.txt
bpf-objdump -rd compiled/atomic.deb13-gccO2-v4.ebpf > atomic.deb13-gccO2-v4.txt

While llvm-objdump produces pseudo-code instructions, such as:

0000000000000000 <atomic_exchange64>:
       0:	79 22 00 00 00 00 00 00	r2 = *(u64 *)(r2 + 0x0)
       1:	db 21 00 00 e1 00 00 00	r2 = xchg_64(r1 + 0x0, r2)
       2:	7b 23 00 00 00 00 00 00	*(u64 *)(r3 + 0x0) = r2
       3:	95 00 00 00 00 00 00 00	exit

... bpf-objdump produces usual ASM mnemonics:

00000000000000b0 <atomic_exchange64>:
  b0:    79 20 00 00 00 00 00 00 	ldxdw %r0,[%r2+0]
  b8:    db 01 00 00 e1 00 00 00 	axchg [%r1+0],%r0
  c0:    7b 03 00 00 00 00 00 00 	stxdw [%r3+0],%r0
  c8:    95 00 00 00 00 00 00 00 	exit

I analyzed the compiled eBPF programs with Ghidra and confirmed the disassembly listing contains (with this Pull Request) similar atomic mnemonics: AXCHG, AFADD, AFOR...

While testing atomic compare-and-exchange, Ghidra showed an unexpected decompiled code. This function

bool atomic_compare_exchange_n64(volatile unsigned long *ptr, unsigned long *expected, unsigned long desired) {
    return __atomic_compare_exchange_n(ptr, expected, desired, 1, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
}

... got decompiled to:

bool atomic_compare_exchange_n64(longlong *param_1,longlong *param_2,longlong param_3)
{
  longlong lVar1;
  longlong lVar2;
  
  lVar1 = *param_2;
  lVar2 = lVar1;
  if (lVar1 != *param_1) {
    lVar2 = *param_1;
  }
  *param_1 = param_3;
  if (lVar2 != lVar1) {
    *param_2 = lVar2;
  }
  return lVar2 == lVar1;
}

The fact that *param_1 = param_3; is always executed is a bug. When the contents of the pointer does not match the expected value, __atomic_compare_exchange_n does not modify this content (and only reads the actual content to expected, which is what the second if statement is about). This Pull Request fixes this bug by adding goto inst_next; where appropriate.

With this, the decompilation becomes:

bool atomic_compare_exchange_n64(longlong *param_1,longlong *param_2,longlong param_3)
{
  longlong lVar1;
  longlong lVar2;
  
  lVar1 = *param_2;
  lVar2 = *param_1;
  if (lVar1 == lVar2) {
    *param_1 = param_3;
    lVar2 = lVar1;
  }
  if (lVar2 != lVar1) {
    *param_2 = lVar2;
  }
  return lVar2 == lVar1;
}

There is a stray statement lVar2 = lVar1; but at least the code is semantically correct.

Attachements

atomic.c

atomic_example.tar.gz

[niooss-ledger]

I found a 2nd bug in the semantics of the 32-bit CMPXCHG instruction: R0 was only updated when R0:4 != tmp, whereas it should always be updated (with R0 = zext(tmp)). This is what eBPF's documentation in Linux kernel states and what the implementation in linux:kernel/bpf/core.c actually does.

So I modified the 2nd commit and force-pushed. With it, the decompilation looks fine:

bool atomic_compare_exchange64(longlong *param_1,longlong *param_2,longlong *param_3)
{
  longlong lVar1;
  longlong lVar2;
  
  lVar2 = *param_2;
  lVar1 = *param_1;
  if (lVar2 == lVar1) {
    *param_1 = *param_3;
  }
  if (lVar1 != lVar2) {
    *param_2 = lVar1;
  }
  return lVar1 == lVar2;
}

And the 32-bit version too:

Source code of `atomic_compare_exchange_n32`

bool atomic_compare_exchange_n32(
    volatile unsigned int *ptr, unsigned int *expected, unsigned int desired
) {
    return __atomic_compare_exchange_n(ptr, expected, desired, 1, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
}