LKML Archive on lore.kernel.org
 help / color / Atom feed
From: "Schmid, Carsten" <Carsten_Schmid@mentor.com>
To: Wei Yang <richard.weiyang@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	"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>,
	"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>,
	Hans de Goede <hdegoede@redhat.com>
Subject: [PATCH] kernel/resource.c: warn if released region has children
Date: Fri, 16 Aug 2019 10:18:02 +0000
Message-ID: <1565950682464.83682@mentor.com> (raw)
In-Reply-To: <ff78412f81494678bb7685dc2604002e@SVR-IES-MBX-03.mgc.mentorg.com>

When a resource region is released and has children,
the children are left without any hint that their
parent is no more valid.

This was observed on a use-after-free fault in the xhci-hcd
when xhci-hcd released his iomem region before
platform code released resources of platform devices
giving a random racy failure; one of the stacks observed:

[  230.412493] xhci_hcd 0000:00:15.0: USB bus 1 deregistered
[  230.416021] general protection fault: 0000 [#1] PREEMPT SMP NOPTI
[  230.422836] Modules linked in: bcmdhd(O-) ebtable_filter ebtables xfrm_user xfrm_algo cls_u32 sch_htb intel_tfm_governor cdc_acm ecryptfs intel_ipu4_psys intel_ipu4_psys_csslib snd_soc_apl_mgu_hu intel_xhci_usb_role_switch roles dwc3 adv728x udc_core snd_soc_skl sdw_cnl intel_ipu4_isys snd_soc_acpi_intel_match videobuf2_dma_contig videobuf2_memops snd_soc_acpi ipu4_acpi intel_ipu4_isys_csslib snd_soc_core snd_compress videobuf2_v4l2 videobuf2_core snd_soc_skl_ipc sdw_bus crc8 snd_soc_sst_ipc snd_soc_sst_dsp ahci snd_hda_ext_core coretemp snd_hda_core sbi_apl intel_ipu4_mmu i2c_i801 snd_pcm xhci_pci snd_timer xhci_hcd libahci cfg80211 mei_me snd usbcore libata intel_ipu4 rfkill soundcore usb_common mei dwc3_pci scsi_mod iova nfsd auth_rpcgss lockd grace sunrpc loop fuse 8021q bridge stp llc inap560t(O)
[  230.502258]  i915 video backlight intel_gtt i2c_algo_bit drm_kms_helper drm firmware_class igb_avb(O) ptp hwmon spi_pxa2xx_platform pps_core [last unloaded: bcmdhd]
[  230.518695] CPU: 1 PID: 20364 Comm: unbind-usb-str. Tainted: G     U     O    4.14.67-apl #1
[  230.528124] task: ffffa106ea869900 task.stack: ffffa24c84970000
[  230.534741] RIP: 0010:__release_resource+0x25/0x90
[  230.540090] RSP: 0018:ffffa24c84973c18 EFLAGS: 00010212
[  230.545924] RAX: f7410001b577e8a8 RBX: ffffa10731b4b700 RCX: ffffa10731b4a6c0
[  230.553893] RDX: f7410001b577e8a8 RSI: 0000000000000001 RDI: ffffa10731b4b700
[  230.561864] RBP: ffffa24c84973c18 R08: 0000000000000000 R09: 0000000000000000
[  230.569825] R10: ffffa24c8493b978 R11: 00000000000006c0 R12: ffffa106f1355000
[  230.577794] R13: ffffa24c84973c98 R14: ffffa24c84973c98 R15: 0000000000000001
[  230.585757] FS:  00007f365c48a740(0000) GS:ffffa10777c80000(0000) knlGS:0000000000000000
[  230.594794] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  230.601203] CR2: 000055562a2f0e88 CR3: 00000001f1108000 CR4: 00000000003406a0
[  230.609174] Call Trace:
[  230.611896]  release_resource+0x21/0x40
[  230.616181]  platform_device_del.part.1+0x4f/0x80
[  230.621433]  platform_device_unregister+0x12/0x20
[  230.626683]  xhci_intel_unregister_pdev+0x9/0x10 [xhci_hcd]
[  230.632906]  devm_action_release+0x10/0x20
[  230.637478]  release_nodes+0x10b/0x1f0
[  230.641663]  devres_release_all+0x37/0x50
[  230.646139]  device_release_driver_internal+0x19d/0x240
[  230.651974]  device_release_driver+0xd/0x10
[  230.656644]  unbind_store+0xb8/0x190
[  230.660633]  drv_attr_store+0x22/0x30
[  230.664720]  sysfs_kf_write+0x37/0x40
[  230.668807]  kernfs_fop_write+0x114/0x190
[  230.673283]  __vfs_write+0x35/0x160
[  230.677176]  vfs_write+0xb0/0x1a0
[  230.680873]  SyS_write+0x50/0xc0
[  230.684476]  do_syscall_64+0x79/0x340
[  230.688566]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[  230.694208] RIP: 0033:0x7f365bb91144
[  230.698198] RSP: 002b:00007ffe3da9eae8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[  230.706655] RAX: ffffffffffffffda RBX: 000000000000000d RCX: 00007f365bb91144
[  230.714628] RDX: 000000000000000d RSI: 000055562a2efe80 RDI: 0000000000000001
[  230.722599] RBP: 000055562a2efe80 R08: 000000000000000a R09: 00007f365be5acd0
[  230.730568] R10: 000000000000000a R11: 0000000000000246 R12: 00007f365be5b760
[  230.738538] R13: 000000000000000d R14: 00007f365be56760 R15: 000000000000000d
[  230.746501] Code: 84 00 00 00 00 00 48 8b 4f 28 55 b8 ea ff ff ff 48 89 e5 48 8b 51 38 48 85 d2 74 1d 48 39 d7 75 0a eb 62 48 39 c7 74 13 48 89 c2 <48> 8b 42 30 48 85 c0 75 ef b8 ea ff ff ff 5d c3 48 83 c2 30 40
[  230.767628] RIP: __release_resource+0x25/0x90 RSP: ffffa24c84973c18

Because the root cause - iomem area was released earlier -
could not be seen on analysis easily, a warning in the
release_region helps to detect such cases (if any exist).

xhci-hcd driver fix is issued separately.

Signed-off-by: Carsten Schmid <carsten_schmid@mentor.com>
Tested-by: Carsten Schmid <carsten_schmid@mentor.com>
---
Rationale:
- Changed title according to reduced functionality.
  Original title:
  kernel/resource.c: invalidate parent when freed resource has childs
- Added more information about why issuing this patch
---
 kernel/resource.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/kernel/resource.c b/kernel/resource.c
index c3cc6d85ec52..0db374029627 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1172,7 +1172,19 @@ EXPORT_SYMBOL(__request_region);
  * @n: resource region size
  *
  * The described resource region must match a currently busy region.
+ * If the region has children warn.
  */
+static void check_children(struct resource *parent)
+{
+	write_lock(&resource_lock);
+
+	if (parent->child)
+		WARN_ONCE(1, "%s: %s still has children, at least %s\n",
+				__func__, parent->name, parent->child->name);
+
+	write_unlock(&resource_lock);
+}
+
 void __release_region(struct resource *parent, resource_size_t start,
 		      resource_size_t n)
 {
@@ -1200,6 +1212,10 @@ void __release_region(struct resource *parent, resource_size_t start,
 			write_unlock(&resource_lock);
 			if (res->flags & IORESOURCE_MUXED)
 				wake_up(&muxed_resource_wait);
+
+			/* You should'nt release a resource that has children */
+			check_children(res);
+
 			free_resource(res);
 			return;
 		}
-- 
2.17.1

      reply index

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 ` Resend " Schmid, Carsten
2019-08-09 16:59   ` 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                 ` Schmid, Carsten [this message]

Reply instructions:

You may reply publically 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=1565950682464.83682@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=hdegoede@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=osalvador@suse.de \
    --cc=rdunlap@infradead.org \
    --cc=richard.weiyang@gmail.com \
    --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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox