On Fri, Aug 09, 2019 at 01:50:24PM +0000, Schmid, Carsten wrote: >When a resource is freed and has children, the childrens are >left without any hint that their parent is no more valid. >This caused at least one use-after-free in the xhci-hcd using >ext-caps driver when platform code released platform devices. > >Fix this by setting child's parent to zero and warn. > >Signed-off-by: Carsten Schmid <[email protected]> >--- >Rationale: >When hunting for the root cause of a crash on a 4.14.86 kernel, i >have found the root cause and checked it being still present >upstream. Our case: >Having xhci-hcd and intel_xhci_usb_sw active we can see in >/proc/meminfo: (exceirpt) > b3c00000-b3c0ffff : 0000:00:15.0 > b3c00000-b3c0ffff : xhci-hcd > b3c08070-b3c0846f : intel_xhci_usb_sw >intel_xhci_usb_sw being a child of xhci-hcd. > >Doing an unbind command >echo 0000:00:15.0 > /sys/bus/pci/drivers/xhci_hcd/unbind >leads to xhci-hcd being freed in __release_region. >The intel_xhci_usb_sw resource is accessed in platform code >in platform_device_del with > for (i = 0; i < pdev->num_resources; i++) { > struct resource *r = &pdev->resource[i]; > if (r->parent) > release_resource(r); > } >as the resource's parent has not been updated, the release_resource >uses the parent: > p = &old->parent->child; >which is now invalid. >Fix this by marking the parent invalid in the child and give a warning >in dmesg. >--- >Advised by Greg (thanks): >Try resending it with at least the people who get_maintainer.pl says has >touched that file last in it. [CS:done] > >Also, Linus is the unofficial resource.c maintainer. I think he has a >set of userspace testing scripts for changes somewhere, so you should > cc: him too. And might as well add me :) [CS:done] > > thanks, > > greg k-h >--- > kernel/resource.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > >diff --git a/kernel/resource.c b/kernel/resource.c >index 158f04ec1d4f..95340cb0b1c2 100644 >--- a/kernel/resource.c >+++ b/kernel/resource.c >@@ -1200,6 +1200,15 @@ void __release_region(struct resource *parent, >resource_size_t start, > write_unlock(&resource_lock); > if (res->flags & IORESOURCE_MUXED) > wake_up(&muxed_resource_wait); >+ >+ write_lock(&resource_lock); >+ if (res->child) {
In theory, child may have siblings. Would it be possible to have several devices under xhci-hcd? >+ printk(KERN_WARNING "__release_region: %s has >child %s," >+ "invalidating childs parent\n", >+ res->name, res->child->name); >+ res->child->parent = NULL; >+ } >+ write_unlock(&resource_lock); > free_resource(res); > return; > } >-- >2.17.1 -- Wei Yang Help you, Help me
