[DPDK/cryptodev Bug 2056] Unsafe RCU usage in cryptodev

[bugzilla@dpdk.org]

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

            Bug ID: 2056
           Summary: Unsafe RCU usage in cryptodev
           Product: DPDK
           Version: 26.11
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: normal
          Priority: Normal
         Component: cryptodev
          Assignee: dev@dpdk.org
          Reporter: stephen@networkplumber.org
  Target Milestone: ---

Summary: cryptodev: callback removal does not wait for readers, RCU reader is
never registered

Component: cryptodev
Version: main (d129414711aa, 2026-09-24); the code dates from the
introduction of enqueue/dequeue callbacks, so all releases since are affected
Severity: normal
Found by: code reading, not reproduced

Description

rte_cryptodev_remove_enq_callback() and rte_cryptodev_remove_deq_callback()
unlink the callback, call rte_rcu_qsbr_synchronize(list->qsbr,
RTE_QSBR_THRID_INVALID) and then rte_free(cb). The synchronize is meant to
wait until no data path thread is still inside the callback chain, but it
returns immediately.

cryptodev_cb_init() creates one rte_rcu_qsbr variable per queue pair and
direction with max_threads = 1. The data path brackets the callback walk in
rte_cryptodev_enqueue_burst() and rte_cryptodev_dequeue_burst() with
rte_rcu_qsbr_thread_online(qsbr, 0) and rte_rcu_qsbr_thread_offline(qsbr, 0).
Nothing ever calls rte_rcu_qsbr_thread_register(qsbr, 0): not lib/cryptodev,
not drivers/crypto, not app/test or examples.

rte_rcu_qsbr_check() takes the __rte_rcu_qsbr_check_all() path only when
num_threads == max_threads. Here num_threads is 0, so it takes
__rte_rcu_qsbr_check_selective(), which iterates the registered-thread bitmap,
finds no bits set and returns 1 without looking at any reader counter.

Result: remove frees the callback while a data path thread can still be
executing it or reading cb->next from it. The online/offline calls on the
data path cost a full barrier per burst and protect nothing.

Expected behaviour

Remove returns only after any in-progress enqueue or dequeue burst on that
queue pair has left the callback chain.

Fix

Register the one reader slot when the variable is created, in
cryptodev_cb_init(), after rte_rcu_qsbr_init():

        rte_rcu_qsbr_thread_register(qsbr, 0);

for both the enq and deq variables. With one registered thread the check
takes the check_all path and waits on the counter the data path updates.

Reproduce

Not done. Suggested: one lcore looping rte_cryptodev_enqueue_burst() on a
queue pair with a callback that spins for a few microseconds, and a control
thread looping add/remove of that callback. Build with ASan; a read of the
freed cb in the burst loop should be reported within seconds.

Related, same code

- Once the wait is real, rte_cryptodev_callback_lock, a global spinlock, is
  held across the synchronize in both remove functions, so add and remove on
  every device stall behind one removal.
- rte_cryptodev_remove_deq_callback() tests dev->enq_cbs == NULL where it
  means deq_cbs. Harmless, both are allocated together.
- rte_cryptodev_close() does not call cryptodev_cb_cleanup(); the lists, qsbr
  variables and callbacks stay allocated until the next configure.

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