]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
net/mlx5e: Fix HV VHCA stats agent registration race
authorFeng Liu <feliu@nvidia.com>
Tue, 30 Jun 2026 11:51:50 +0000 (14:51 +0300)
committerPaolo Abeni <pabeni@redhat.com>
Fri, 3 Jul 2026 16:50:31 +0000 (18:50 +0200)
mlx5e_hv_vhca_stats_create() registers the stats agent through
mlx5_hv_vhca_agent_create(). The helper publishes the agent in
hv_vhca->agents[type] under agents_lock and immediately schedules an
asynchronous control invalidation on the HV VHCA workqueue before
returning to mlx5e.

The asynchronous invalidation invokes the control agent's invalidate
callback, which reads the hypervisor control block and forwards the
command to mlx5e_hv_vhca_stats_control(). That callback may either:

  - call cancel_delayed_work_sync(&priv->stats_agent.work), or
  - call queue_delayed_work(priv->wq, &sagent->work, sagent->delay).

However, the delayed_work and priv->stats_agent.agent are only
initialized after mlx5_hv_vhca_agent_create() returns to mlx5e:

    agent = mlx5_hv_vhca_agent_create(...);   /* publish + invalidate */
    ...
    priv->stats_agent.agent = agent;          /* too late */
    INIT_DELAYED_WORK(&priv->stats_agent.work, ...); /* too late */

If the asynchronous control path runs before the two assignments
above, it can:

  - Operate on an uninitialized delayed_work whose timer.function is
    NULL. queue_delayed_work() calls add_timer() unconditionally, so
    when the timer expires the timer softirq invokes a NULL function
    pointer.
  - Re-initialize the timer later through INIT_DELAYED_WORK() while
    the timer is already enqueued in the timer wheel, corrupting the
    hlist (entry.pprev cleared while the previous bucket node still
    points at this entry).
  - When the worker eventually runs, mlx5e_hv_vhca_stats_work() reads
    sagent->agent (NULL) and dereferences it inside
    mlx5_hv_vhca_agent_write().

Fix this by:

  - Initializing priv->stats_agent.work before invoking
    mlx5_hv_vhca_agent_create(), so the work is always in a valid
    state when the control callback observes it.
  - Adding a struct mlx5_hv_vhca_agent **ctx_update out-parameter
    to mlx5_hv_vhca_agent_create(). The helper writes the agent
    pointer to *ctx_update before publishing into hv_vhca->agents[]
    and triggering the agents_update flow, so any callback
    subsequently invoked from that flow already sees a valid
    priv->stats_agent.agent. This avoids having the control
    callback participate in agent initialization.

While at it, access priv->stats_agent.agent with
READ_ONCE()/WRITE_ONCE() for the cross-CPU access with the worker, and
clear priv->stats_agent.buf on the agent_create() failure path.

Fixes: cef35af34d6d ("net/mlx5e: Add mlx5e HV VHCA stats agent")
Signed-off-by: Feng Liu <feliu@nvidia.com>
Reviewed-by: Eran Ben Elisha <eranbe@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Link: https://patch.msgid.link/20260630115151.729219-3-tariqt@nvidia.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h

index 72f3ca4dd076bbde82b5c18285d0c4aae974b480..cdaf77650164b972bc6ec53618f31604905ee439 100644 (file)
@@ -73,7 +73,7 @@ static void mlx5e_hv_vhca_stats_work(struct work_struct *work)
        sagent = container_of(dwork, struct mlx5e_hv_vhca_stats_agent, work);
        priv = container_of(sagent, struct mlx5e_priv, stats_agent);
        buf_len = mlx5e_hv_vhca_stats_buf_size(priv);
-       agent = sagent->agent;
+       agent = READ_ONCE(sagent->agent);
        buf = sagent->buf;
 
        memset(buf, 0, buf_len);
@@ -135,11 +135,14 @@ void mlx5e_hv_vhca_stats_create(struct mlx5e_priv *priv)
        if (!priv->stats_agent.buf)
                return;
 
+       INIT_DELAYED_WORK(&priv->stats_agent.work, mlx5e_hv_vhca_stats_work);
+
        agent = mlx5_hv_vhca_agent_create(priv->mdev->hv_vhca,
                                          MLX5_HV_VHCA_AGENT_STATS,
                                          mlx5e_hv_vhca_stats_control, NULL,
                                          mlx5e_hv_vhca_stats_cleanup,
-                                         priv);
+                                         priv,
+                                         &priv->stats_agent.agent);
 
        if (IS_ERR_OR_NULL(agent)) {
                if (IS_ERR(agent))
@@ -148,20 +151,20 @@ void mlx5e_hv_vhca_stats_create(struct mlx5e_priv *priv)
                                    agent);
 
                kvfree(priv->stats_agent.buf);
-               return;
+               priv->stats_agent.buf = NULL;
        }
-
-       priv->stats_agent.agent = agent;
-       INIT_DELAYED_WORK(&priv->stats_agent.work, mlx5e_hv_vhca_stats_work);
 }
 
 void mlx5e_hv_vhca_stats_destroy(struct mlx5e_priv *priv)
 {
-       if (IS_ERR_OR_NULL(priv->stats_agent.agent))
+       struct mlx5_hv_vhca_agent *agent;
+
+       agent = READ_ONCE(priv->stats_agent.agent);
+       if (IS_ERR_OR_NULL(agent))
                return;
 
-       mlx5_hv_vhca_agent_destroy(priv->stats_agent.agent);
-       priv->stats_agent.agent = NULL;
+       mlx5_hv_vhca_agent_destroy(agent);
+       WRITE_ONCE(priv->stats_agent.agent, NULL);
        kvfree(priv->stats_agent.buf);
        priv->stats_agent.buf = NULL;
 }
index d6dc7bce855e0bebbb8a4cf195230ae097fe7808..305752dab7bdf4fc027ff784065a309092611314 100644 (file)
@@ -190,7 +190,7 @@ mlx5_hv_vhca_control_agent_create(struct mlx5_hv_vhca *hv_vhca)
        return mlx5_hv_vhca_agent_create(hv_vhca, MLX5_HV_VHCA_AGENT_CONTROL,
                                         NULL,
                                         mlx5_hv_vhca_control_agent_invalidate,
-                                        NULL, NULL);
+                                        NULL, NULL, NULL);
 }
 
 static void mlx5_hv_vhca_control_agent_destroy(struct mlx5_hv_vhca_agent *agent)
@@ -256,7 +256,8 @@ mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
                          void (*invalidate)(struct mlx5_hv_vhca_agent*,
                                             u64 block_mask),
                          void (*cleaup)(struct mlx5_hv_vhca_agent *agent),
-                         void *priv)
+                         void *priv,
+                         struct mlx5_hv_vhca_agent **ctx_update)
 {
        struct mlx5_hv_vhca_agent *agent;
 
@@ -284,6 +285,9 @@ mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
        agent->invalidate = invalidate;
        agent->cleanup   = cleaup;
 
+       if (ctx_update)
+               WRITE_ONCE(*ctx_update, agent);
+
        mutex_lock(&hv_vhca->agents_lock);
        hv_vhca->agents[type] = agent;
        mutex_unlock(&hv_vhca->agents_lock);
index f240ffe5116c5b86dd83c5013e01d3cd52ba4962..8b3974cf0ee49afbebba989aba6276b029930279 100644 (file)
@@ -43,7 +43,8 @@ mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
                          void (*invalidate)(struct mlx5_hv_vhca_agent*,
                                             u64 block_mask),
                          void (*cleanup)(struct mlx5_hv_vhca_agent *agent),
-                         void *context);
+                         void *context,
+                         struct mlx5_hv_vhca_agent **ctx_update);
 
 void mlx5_hv_vhca_agent_destroy(struct mlx5_hv_vhca_agent *agent);
 int mlx5_hv_vhca_agent_write(struct mlx5_hv_vhca_agent *agent,
@@ -84,7 +85,8 @@ mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
                          void (*invalidate)(struct mlx5_hv_vhca_agent*,
                                             u64 block_mask),
                          void (*cleanup)(struct mlx5_hv_vhca_agent *agent),
-                         void *context)
+                         void *context,
+                         struct mlx5_hv_vhca_agent **ctx_update)
 {
        return NULL;
 }