The DQO TX path uses lockless producer/consumer free lists
(free_pending_pkts_prd/csm and free_qpl_bufs_prd/csm) shared between
the completion and transmit taskqueues. On weakly ordered architectures
such as arm64, the current synchronization has three issues:
- gve_free_pending_packet() updates pending_pkt state and links pending_pkt->next before pushing the entry onto free_pending_pkts_prd using a relaxed atomic_cmpset_32(), while gve_alloc_pending_packet() steals free_pending_pkts_prd using a relaxed atomic_swap_32(). Without release/acquire ordering, the transmit thread can observe a stale pending_pkt->next pointer after stealing the producer list (either orphaning freed entries or re-allocating entries whose completion tags are still in flight), or prior completion stores (such as setting state to FREE or clearing mbuf) can become visible after the slot is re-allocated.
- Similarly, in QPL mode, gve_tx_alloc_qpl_buf() steals free_qpl_bufs_prd with a relaxed atomic_swap_32() without an acquire fence to pair with atomic_cmpset_rel_32() in gve_reap_qpl_bufs_dqo().
- gve_alloc_pending_packet() marks pending_pkt->state as GVE_PACKET_STATE_PENDING_DATA_COMPL and sets the enqueue timestamp before pkt->mbuf is populated and before fallible checks in the transmit path succeed. If a completion arrives for a duplicate tag (from issue 1) while the second transmit is still in progress or after it aborts, gve_handle_packet_completion() observes a pending state while pkt->mbuf is NULL.
This was discovered during multi-stream TCP testing on ARM64 GCP
instances running FreeBSD 14.3-RELEASE, where it caused spurious
missing-completion logs followed by a NULL pointer dereference of
pending_pkt->mbuf (offset 0x38) in gve_handle_packet_completion():
gve0: No pending data completion: 0 gve0: No pending data completion: 1 gve0: No pending data completion: 2 Fatal data abort: x0: 0xffffe00009d61e00 x1: 0xffffe00009ce6580 ... elr: 0xffff0000013b49d8 (gve_tx_cleanup_tq_dqo + 0x120) far: 0x0000000000000038 panic: vm_fault failed: 0xffff0000013b49d8 error 1 cpuid = 14 KDB: stack backtrace: #0 0xffff000000516174 at kdb_backtrace+0x60 #1 0xffff0000004bc4c0 at vpanic+0x13c #2 0xffff0000004bc380 at panic+0x44 #3 0xffff0000008d22bc at data_abort+0x314 #4 0xffff0000008b1878 at handle_el1h_sync+0x18 #5 0xffff0000013b49d8 at gve_tx_cleanup_tq_dqo+0x120 #6 0xffff00000052d4f0 at taskqueue_run_locked+0x17c #7 0xffff00000052e4f0 at taskqueue_thread_loop+0xc8
Use atomic_cmpset_rel_32() when pushing onto free_pending_pkts_prd, add
atomic_thread_fence_acq() after stealing free_pending_pkts_prd in
gve_alloc_pending_packet() and free_qpl_bufs_prd in
gve_tx_alloc_qpl_buf(), clear pending_pkt->next upon allocation, and
defer populating pkt->mbuf, pkt->state, and the enqueue timestamp until
all fallible checks in the transmit path have succeeded.
Additionally, replace atomic_load_acq_32() with a relaxed
atomic_load_32() when reading old_head in gve_free_pending_packet(), as
old_head is only used as a scalar index and never dereferenced by the
producer.
Fixes: d438b4ef0cfc ("gve: Add DQO RDA support")
Fixes: 2348ac893d10 ("gve: Add DQO QPL support")
Reviewed-by: Jasper Tran O'Leary <jtranoleary@google.com>
Signed-off-by: Tim Hostetler <thostet@google.com>