Re: [PATCH net-next v5 10/12] gve: setup and teardown management interrupts

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The `stats_report_task` is not canceled during device removal if the network interface was down, leading to a NULL pointer dereference when the workqueue is destroyed.
--

commit b267ee52d04bbd318d288ea0576f1ecaaa85d7ae
Author: Joshua Washington <joshwash@google.com>

gve: setup and teardown management interrupts

This patch introduces control ops to set up and tear down control plane IRQs.
It moves the admin queue-specific functionality to the admin queue files.

> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index a160d36f718b9..8522a495b78aa 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -290,18 +290,6 @@ static void gve_free_stats_report(struct gve_priv *priv)
>  	priv->stats_report = NULL;
>  }

[Severity: High]
This is a pre-existing issue, but since the stats report is freed here, can
the subsequent workqueue destruction in gve_remove() cause a NULL pointer
dereference?

If the network interface was down, unregister_netdev() bypasses gve_close(),
omitting the cancellation of stats_report_task. The stats_report_task can be
queued by gve_stats_report_timer() if the timer executes just before
timer_delete_sync() runs in gve_free_stats_report().

Because gve_free_stats_report() frees priv->stats_report and sets the pointer
to NULL, when gve_remove() later calls:

    disable_work_sync(&priv->service_task);
    gve_teardown_device(priv);
    destroy_workqueue(priv->gve_wq);

The destroy_workqueue() call synchronously flushes and executes the pending
stats_report_task. This executes gve_handle_report_stats(), which does:

    struct stats *stats = priv->stats_report->stats;

This unconditionally dereferences the NULL priv->stats_report pointer, leading
to a system crash. Should stats_report_task be explicitly canceled in
gve_remove() before destroying the workqueue?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903215606.31633-1-hramamurthy@google.com?part=10