Re: [PATCH] PCI / PM: Allow runtime PM without callback functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/20/2018 07:19 PM, Bjorn Helgaas wrote:
On Fri, Oct 19, 2018 at 03:21:05PM +0200, Jean Delvare wrote:
Later in this function, pm is dereferenced again. It happens twice in
the "if (error)" condition where it is currently safe (error can't be
non-zero if pm->runtime_suspend() has not been called, and obviously
pm->runtime_suspend() can't have been called if pm was NULL). However
it also happens later without the condition:

	if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
	    && pci_dev->current_state != PCI_UNKNOWN) {
		WARN_ONCE(pci_dev->current_state != prev,
			"PCI PM: State of device not saved by %pF\n",
			pm->runtime_suspend);
		return 0;
	}

I am no expert of the PM framework but is there no risk to dereference
NULL at this point? Or even if pm is non-NULL, pm->runtime_suspend may
be NULL, leading to a confusing warning message?

Thanks for spotting this! I don't have any excuse why I was so completely blind.

More generally, I would feel better if instead of initializing error to
0, we would move under the "if (pm && pm->runtime_suspend)" condition
everything that must not be run if pm->runtime_suspend is not defined.
That would make the possible code flows a lot clearer.

I agree, this isn't good.  Even if it's safe (and I don't think that
second spot is safe), it's too hard to analyze.  I'm going to drop
this for now.

Thanks. I'll cook a better version.

--
Jarkko



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux