Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1623475

Re: [PATCH V8 3/5] PCI/ASPM: add init hook to device_add

From Bjorn Helgaas <bhelgaas@google.com>
Newsgroups linux.kernel
Subject Re: [PATCH V8 3/5] PCI/ASPM: add init hook to device_add
Date 2017-04-14 03:40 +0200
Message-ID <tvYnn-2Ws-1@gated-at.bofh.it> (permalink)
References <ttQDD-FB-3@gated-at.bofh.it> <ttQDE-FB-13@gated-at.bofh.it> <tvTQK-8fn-27@gated-at.bofh.it> <tvUa6-b9-19@gated-at.bofh.it> <tvY42-2OS-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, Apr 13, 2017 at 8:19 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
> On 4/13/2017 5:02 PM, Bjorn Helgaas wrote:
>> I do see that you change the deallocation in patch [5/5], but I think
>> the deallocation change should be in the same patch as the allocation
>> change.  Otherwise I think we have a use-after-free problem in this
>> sequence:
>
> Sure, I'll reorder. As you can see here, link will be only removed if
> root port is being removed.
>
> Without this, we'll hit the use after free issue you mentioned.
>
>         if (pdev->has_secondary_link) {
>                 link = pdev->link_state;
>                 down_read(&pci_bus_sem);
>                 mutex_lock(&aspm_lock);
>
>                 list_del(&link->sibling);
>                 list_del(&link->link);
>
>                 /* Clock PM is for endpoint device */
>                 free_link_state(link);
>                 mutex_unlock(&aspm_lock);
>                 up_read(&pci_bus_sem);
>                 return;
>         }

Right, but this "pdev->has_secondary_link" check is in your new code
and doesn't show up until patch [5/5].

As of *this* patch [3/5], the link is removed when the endpoint is
removed, so we could hit the use-after-free.

Granted, we'll only be susceptible to this while bisecting because
normally all the patches will be applied.  But I think this patch will
make more sense and be easier to review if it makes the link_state
lifetime match the Port's lifetime.

Bjorn

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

Re: [PATCH V8 3/5] PCI/ASPM: add init hook to device_add Bjorn Helgaas <helgaas@kernel.org> - 2017-04-13 22:50 +0200
  Re: [PATCH V8 3/5] PCI/ASPM: add init hook to device_add Bjorn Helgaas <helgaas@kernel.org> - 2017-04-13 23:10 +0200
    Re: [PATCH V8 3/5] PCI/ASPM: add init hook to device_add Sinan Kaya <okaya@codeaurora.org> - 2017-04-14 03:20 +0200
      Re: [PATCH V8 3/5] PCI/ASPM: add init hook to device_add Bjorn Helgaas <bhelgaas@google.com> - 2017-04-14 03:40 +0200

csiph-web