lkml.org 
[lkml]   [2017]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH V8 3/5] PCI/ASPM: add init hook to device_add
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

\
 
 \ /
  Last update: 2017-04-14 03:32    [W:0.051 / U:1.320 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site