Re: [PATCH iwl-net 09/10] ice: program multicast magic wake before tearing down the main VSI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ice: Use-After-Free in ice_setup_mc_magic_wake() during concurrent devlink reload
--

commit 6cf8b519dd4de7f3996192dbff4ffe7afc4d34f8
Author: Petr Oros <poros@redhat.com>

ice: program multicast magic wake before tearing down the main VSI

This commit moves the ice_setup_mc_magic_wake() call earlier in ice_remove()
before device structures are torn down. It fixes an issue where WoL magic
packet wake was not set up correctly because it was being called after
ice_deinit().

> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index b121faa345c1a..857ba7d92b140 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -5465,6 +5465,8 @@ static void ice_remove(struct pci_dev *pdev)
>  	if (!ice_is_safe_mode(pf))
>  		ice_remove_arfs(pf);
>  
> +	ice_setup_mc_magic_wake(pf);
> +
>  	devl_lock(priv_to_devlink(pf));

[Severity: High]
By moving ice_setup_mc_magic_wake() outside of devl_lock(), is it exposed to
concurrent devlink operations like reload?

If a devlink reload operation executes concurrently with the driver's removal
routine when Wake-on-LAN is enabled, the reload can call ice_unload() and
ice_decfg_netdev(). This unregisters and frees the netdev.

When ice_setup_mc_magic_wake() runs unprotected by the devlink lock, can it
experience a use-after-free when attempting to read the freed netdev's MAC
address?

drivers/net/ethernet/intel/ice/ice_main.c:ice_setup_mc_magic_wake() {
    ...
	if (vsi->netdev)
		ether_addr_copy(mac_addr, vsi->netdev->dev_addr);
    ...
}

Would it be safer to place the ice_setup_mc_magic_wake() call after acquiring
the devlink lock?

>  	ice_dealloc_all_dynamic_ports(pf);
>  	ice_deinit_devlink(pf);
> @@ -5475,7 +5477,6 @@ static void ice_remove(struct pci_dev *pdev)
>  	ice_deinit(pf);
>  	ice_vsi_release_all(pf);
>  
> -	ice_setup_mc_magic_wake(pf);
>  	ice_set_wake(pf);
>  
>  	ice_adapter_put(pdev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002130752.2597259-1-poros@redhat.com?part=9