[DPDK/ethdev Bug 2055] bpf: missing acquire barrier in bpf_eth_cbi_wait() allows use-after-free on unload

[DPDK/ethdev Bug 2055] bpf: missing acquire barrier in bpf_eth_cbi_wait() allows use-after-free on unload

[bugzilla]

https://bugs.dpdk.org/show_bug.cgi?id=2055

            Bug ID: 2055
           Summary: bpf: missing acquire barrier in bpf_eth_cbi_wait()
                    allows use-after-free on unload
           Product: DPDK
           Version: 26.11
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: normal
          Priority: Normal
         Component: ethdev
          Assignee: dev@dpdk.org
          Reporter: qm2k21@gmail.com
  Target Milestone: ---

In bpf_pkt.c:103-117, bpf_eth_cbi_unload() synchronizes with datapath threads
before destroying the BPF program:
```
static void     
bpf_eth_cbi_unload(struct bpf_eth_cbi *bc)
{                       
        /* mark this cbi as empty */
        bc->cb = NULL;
        rte_smp_mb();

        /* make sure datapath doesn't use bpf anymore, then destroy bpf */
        bpf_eth_cbi_wait(bc);
        rte_bpf_destroy(bc->bpf);
        bpf_eth_cbi_cleanup(bc); 
}                       
```

The synchronization in bpf_eth_cbi_wait() polls cbi->use:
```
static void
bpf_eth_cbi_wait(const struct bpf_eth_cbi *cbi)
{
        uint32_t puse;

        /* make sure all previous loads and stores are completed */
        rte_smp_mb();

        puse = cbi->use;

        /* in use, busy wait till current RX/TX iteration is finished */
        if ((puse & BPF_ETH_CBI_INUSE) != 0) {
                RTE_WAIT_UNTIL_MASKED((__rte_atomic uint32_t
*)(uintptr_t)&cbi->use,
                        UINT32_MAX, !=, puse, rte_memory_order_relaxed);
        }
}               
```

While bpf_eth_cbi_wait() begins with a full barrier (rte_smp_mb()), that
barrier precedes reading cbi->use. Its sole purpose is to ensure the store
bc->cb = NULL is visible before reading cbi->use.     
When waiting for an in-flight callback to finish, RTE_WAIT_UNTIL_MASKED polls
cbi->use with rte_memory_order_relaxed.
When the condition is met and the loop terminates, bpf_eth_cbi_wait() returns
without any acquire barrier (neither an acquire load nor an acquire/read
fence).
Control plane execution immediately proceeds to rte_bpf_destroy(bc->bpf), which
frees the memory of the BPF program (rte_free(), munmap()).
On weakly ordered architectures (ARM64, PowerPC, RISC-V), without an acquire
barrier observing the datapath's completion, the CPU can speculatively execute
or reorder rte_bpf_destroy() ahead of the datapath completing its memory
accesses into the BPF filter code/context. This produces a race condition and a
use-after-free.

One might assume that because RTE_WAIT_UNTIL_MASKED invokes rte_pause() inside
its spin loop, it might provide memory ordering. This assumption is incorrect
for several reasons:
* semantics of rte_pause() is not explicitly documented, and on some
architectures it does not provide a sufficient barrier (e.g. on arm32 it is an
empty function);
* semantics of RTE_WAIT_UNTIL_MASKED is documented, and explicitly warns that
"With a relaxed memory ordering model, the loads around this helper can be
reordered";
* last but not least, rte_pause() is only executed between the failed
iterations, but not after the last (successful) one -- even with rte_pause()
forming the memory barrier a small gap for reordering still exists.

-- 
You are receiving this mail because:
You are the assignee for the bug.