All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Schmid, Carsten" <Carsten_Schmid@mentor.com>
To: "bp@suse.de" <bp@suse.de>,
	"dan.j.williams@intel.com" <dan.j.williams@intel.com>,
	"mingo@kernel.org" <mingo@kernel.org>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "bhelgaas@google.com" <bhelgaas@google.com>,
	"osalvador@suse.de" <osalvador@suse.de>,
	"rdunlap@infradead.org" <rdunlap@infradead.org>,
	"richardw.yang@linux.intel.com" <richardw.yang@linux.intel.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"torvalds@linux-foundation.org" <torvalds@linux-foundation.org>
Subject: Resend [PATCH] kernel/resource.c: invalidate parent when freed resource has childs
Date: Fri, 9 Aug 2019 13:50:24 +0000	[thread overview]
Message-ID: <1565358624103.3694@mentor.com> (raw)
In-Reply-To: <1565278859475.1962@mentor.com>

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]

 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) {
+                               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

  reply	other threads:[~2019-08-09 13:50 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-08 15:40 [PATCH] kernel/resource.c: invalidate parent when freed resource has childs Schmid, Carsten
2019-08-09 13:50 ` Schmid, Carsten [this message]
2019-08-09 16:59   ` Resend " Dan Williams
2019-08-09 18:37   ` Joe Perches
2019-08-09 18:54     ` [PATCH] kernel/resource.c: Convert printks to pr_<level> Joe Perches
2019-08-09 20:09   ` Resend [PATCH] kernel/resource.c: invalidate parent when freed resource has childs Linus Torvalds
2019-08-09 22:38   ` Wei Yang
2019-08-09 22:45     ` Linus Torvalds
2019-08-10  0:44       ` Wei Yang
2019-08-12  8:39         ` AW: " Schmid, Carsten
2019-08-13  8:09       ` Schmid, Carsten
2019-08-14 14:48       ` [PATCH v2] " Schmid, Carsten
2019-08-14 16:29         ` Wei Yang
2019-08-15  8:18           ` AW: " Schmid, Carsten
2019-08-15 13:03             ` Wei Yang
2019-08-15 13:17               ` AW: " Schmid, Carsten
2019-08-16 10:18                 ` [PATCH] kernel/resource.c: warn if released region has children Schmid, Carsten

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1565358624103.3694@mentor.com \
    --to=carsten_schmid@mentor.com \
    --cc=bhelgaas@google.com \
    --cc=bp@suse.de \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=osalvador@suse.de \
    --cc=rdunlap@infradead.org \
    --cc=richardw.yang@linux.intel.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.