On 2/26/26 11:40 AM, Eelco Chaudron via dev wrote:
> Coverity reports a data race where pmd_alloc_static_tx_qid() accesses
> pmd->static_tx_qid in the VLOG_DBG() call without holding the
> tx_qid_pool_mutex, while id_pool_alloc_id() writes to static_tx_qid
> with the mutex held.
>
> The race occurs because the VLOG_DBG() call is placed after the mutex
> unlock. While this is a debug-only log with minimal practical impact,
> it violates proper locking discipline and could theoretically read a
> stale or inconsistent value.
>
> Fix by moving the VLOG_DBG() call before the mutex unlock, ensuring
> the read of static_tx_qid is protected by the same lock that protects
> the write.
>
> Fixes: 140dd699463a ("dpif-netdev: Incremental addition/deletion of PMD
> threads.")
> Signed-off-by: Eelco Chaudron <[email protected]>
> ---
> lib/dpif-netdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 9df05c4c2..54f3c8c56 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -6466,10 +6466,10 @@ pmd_alloc_static_tx_qid(struct dp_netdev_pmd_thread
> *pmd)
> VLOG_ABORT("static_tx_qid allocation failed for PMD on core %2d"
> ", numa_id %d.", pmd->core_id, pmd->numa_id);
> }
> - ovs_mutex_unlock(&pmd->dp->tx_qid_pool_mutex);
>
> VLOG_DBG("static_tx_qid = %d allocated for PMD thread on core %2d"
> ", numa_id %d.", pmd->static_tx_qid, pmd->core_id,
> pmd->numa_id);
> + ovs_mutex_unlock(&pmd->dp->tx_qid_pool_mutex);
> }
>
> static void
This doesn't look right. The mutex is protecting the pool,
not the variable. THe variable is also only ever accessed
from the PMD thread itself, so doesn't need locking.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
