Re: [PATCH net-next v5 08/15] net: ethernet: oa_tc6: read, write interface with MMS option

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incorrect teardown order in lan865x_remove allows a race condition leading to a use-after-free on the private netdev structure.
--

commit 6ac71b4b2fa47efef74336ba2f488f113f5cdd43
Author: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>

net: ethernet: oa_tc6: read, write interface with MMS option

This commit introduces new read/write APIs to the oa_tc6 framework that
accept the Memory Map Selector (MMS) as a parameter. This allows vendors
to use any memory map selector without having to manually encode the
address prior to calling the API. The lan865x driver is updated to use
these new functions.

> diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c
> index 0277d9737369a..3b555ee698041 100644
> --- a/drivers/net/ethernet/microchip/lan865x/lan865x.c
> +++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c

[ ... ]

> @@ -235,7 +240,8 @@ static void lan865x_multicast_work_handler(struct work_struct *work)
>  		if (lan865x_clear_all_multicast_addr(priv))
>  			return;
>  	}
> -	ret = oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_NET_CFG, regval);
> +	ret = oa_tc6_write_register_mms(priv->tc6, LAN865X_REG_MAC_NET_CFG,
> +					OA_TC6_PHY_C45_MAC_MMS1, regval);
>  	if (ret)
>  		netdev_err(priv->netdev, "Failed to enable promiscuous/multicast/normal mode: %d\n",
>  			   ret);

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the changes
around the multicast work handler, I noticed an issue with the teardown order
in lan865x_remove().

Can a use-after-free occur here?

drivers/net/ethernet/microchip/lan865x/lan865x.c:lan865x_remove() {
    ...
    cancel_work_sync(&priv->multicast_work);
    unregister_netdev(priv->netdev);
    oa_tc6_exit(priv->tc6);
    free_netdev(priv->netdev);
}

Since cancel_work_sync() is called before unregister_netdev(), couldn't
user-space trigger a configuration change (like toggling promiscuity via
ip link) during this window?

This would call ndo_set_rx_mode and reschedule the multicast work.

When free_netdev() subsequently frees the priv structure, the queued work
would remain on the workqueue, potentially accessing the freed priv pointer
when it later executes.

Should unregister_netdev() be called before cancel_work_sync() so the device
is no longer visible to user-space, preventing further ndo_set_rx_mode calls?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260614-s2500-mac-phy-support-v5-0-89874b72f725@onsemi.com?part=8