All of lore.kernel.org
 help / color / mirror / 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	[thread overview]
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	other threads:[~2019-08-16 10:18 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 ` 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 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=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
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.