Re: [PATCH] net: mana: Clear RDMA teardown and suspend state in mana_rdma_probe()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lockless access to `gd->adev` during probe causes hardware SUSPEND events to be silently dropped, leading to a state mismatch between the host and hardware.
--

commit a413863cad4d3b91f96dce2e01b3fa3e47e6054e
Author: Long Li <longli@microsoft.com>

net: mana: Clear RDMA teardown and suspend state in mana_rdma_probe()

This commit clears gd->rdma_teardown and gd->is_suspended during
mana_rdma_probe() to allow servicing events to be handled again after a reset
cycle. It uses smp_store_release() to publish the state before adding the
auxiliary device.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 7a1ac853e3abc..56f426da72864 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c

[ ... ]

> @@ -4279,6 +4280,17 @@ int mana_rdma_probe(struct gdma_dev *gd)
>  	if (err)
>  		return err;
>  
> +	/* Clear the state left by a previous mana_rdma_remove() so servicing
> +	 * events are handled again after a reset cycle.
> +	 */
> +	gd->is_suspended = false;
> +
> +	/* Publish is_suspended before re-opening the gate, so the handler
> +	 * cannot act on a stale value.  Pairs with the smp_load_acquire()
> +	 * in mana_rdma_service_handle().
> +	 */
> +	smp_store_release(&gd->rdma_teardown, false);
> +
>  	err = add_adev(gd, "rdma");

[Severity: High]
Could clearing the teardown flag before the auxiliary device is fully
initialized cause hardware suspend events to be silently dropped?

If a GDMA_SERVICE_TYPE_RDMA_SUSPEND event arrives from hardware immediately
after smp_store_release() clears gd->rdma_teardown, the handler in
mana_rdma_service_handle() will be able to process it.

However, because add_adev() has not yet completed and published gd->adev,
the lockless check in mana_rdma_service_handle() will see a NULL pointer:

mana_rdma_service_handle() {
        ...
        switch (serv_work->event) {
        case GDMA_SERVICE_TYPE_RDMA_SUSPEND:
                if (!gd->adev || gd->is_suspended)
                        break;
        ...
}

Will this result in the handler breaking out early and dropping the suspend
event locklessly, leading to a state mismatch between the host and hardware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826235940.1869565-1-longli@microsoft.com?part=1