Messages in this thread Patch in this message | | | Subject | Re: Resend [PATCH] kernel/resource.c: invalidate parent when freed resource has childs | From | Joe Perches <> | Date | Fri, 09 Aug 2019 11:37:31 -0700 |
| |
On Fri, 2019-08-09 at 13:50 +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 <carsten_schmid@mentor.com> > --- > 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] [] > diff --git 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) { > + printk(KERN_WARNING "__release_region: %s has child %s," > + "invalidating childs parent\n", > + res->name, res->child->name);
Please coalesce the format because there is likely an unintentional missing space after the comma, and use pr_warn, %s and __func__
pr_warn("%s: %s has child %s, invalidating child's parent\n", __func__, res->name, res->child->name);
| |