netdev - Re: [net-next 04/15] ice: Add advanced power mgmt for WoL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <edf9b3af-86b0-496b-a306-5d61e2abc092@intel.com>
Date: Thu, 7 Aug 2025 11:21:46 -0700
From: Tony Nguyen <anthony.l.nguyen@...el.com>
To: Lukas Wunner <lukas@...ner.de>
CC: <davem@...emloft.net>, <netdev@...r.kernel.org>, <nhorman@...hat.com>,
	<sassmann@...hat.com>, Intel Wired LAN <intel-wired-lan@...ts.osuosl.org>
Subject: Re: [net-next 04/15] ice: Add advanced power mgmt for WoL



On 8/6/2025 11:20 PM, Lukas Wunner wrote:
> On Thu, Jul 23, 2020 at 04:47:09PM -0700, Tony Nguyen wrote:
>> From: Akeem G Abodunrin <akeem.g.abodunrin@...el.com>
>>
>> Add callbacks needed to support advanced power management for Wake on LAN.
>> Also make ice_pf_state_is_nominal function available for all configurations
>> not just CONFIG_PCI_IOV.
> 
> The above was applied as commit 769c500dcc1e.
> 
>> +static int ice_resume(struct device *dev)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	enum ice_reset_req reset_type;
>> +	struct ice_pf *pf;
>> +	struct ice_hw *hw;
>> +	int ret;
>> +
>> +	pci_set_power_state(pdev, PCI_D0);
>> +	pci_restore_state(pdev);
>> +	pci_save_state(pdev);
>> +
>> +	if (!pci_device_is_present(pdev))
>> +		return -ENODEV;
>> +
>> +	ret = pci_enable_device_mem(pdev);
>> +	if (ret) {
>> +		dev_err(dev, "Cannot enable device after suspend\n");
>> +		return ret;
>> +	}
> 
> You're calling pci_enable_device_mem() on resume without having called
> pci_disable_device() on suspend.  This leads to an imbalance of the
> enable_cnt kept internally in the PCI core.
> 
> Every time you suspend, the enable_cnt keeps growing.
> 
> The user-visible effect is that if you suspend the device at least once
> and then unbind the driver, pci_disable_device() isn't called because
> the enable_cnt hasn't reached zero (and will never reach it again).
> 
> I recommend removing the call to pci_enable_device_mem() in ice_resume():
> The call to pci_restore_state() should already be sufficient to set the
> Memory Space bit in the Command register again on resume.
> 
> I cannot test this for lack of hardware but can provide a patch if you
> want me to.

Hi Lukas,

Thanks for finding this. If you'd like to submit a patch for this, I can 
request our validation test it.

Thanks,
Tony