All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Christian Kellner <ckellner@redhat.com>,
	Andreas Noever <andreas.noever@gmail.com>,
	Michael Jamet <michael.jamet@intel.com>,
	Yehezkel Bernat <yehezkel.bernat@intel.com>,
	Lukas Wunner <lukas@wunner.de>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2] thunderbolt: Do not enumerate more ports from DROM than the controller has
Date: Tue, 25 Jul 2017 17:41:58 +0300	[thread overview]
Message-ID: <20170725144158.23806-1-mika.westerberg@linux.intel.com> (raw)

Some Alpine Ridge LP DROMs (there might be others) erroneusly list more
ports than the controller actually has. Most probably because DROM of
the full Dual/Single port Thunderbolt controller was reused for LP
version. The current DROM parser does not check the upper bound thus it
leads to crash when sw->ports[] is accessed over bounds:

 BUG: unable to handle kernel NULL pointer dereference at 00000000000002ec
 IP: tb_drom_read+0x383/0x890 [thunderbolt]
 PGD 0
 P4D 0
 Oops: 0000 [#1] SMP
 CPU: 3 PID: 12248 Comm: systemd-udevd Not tainted 4.13.0-rc1-next-20170719 #1
 Hardware name: LENOVO 20HF000YGE/20HF000YGE, BIOS N1WET32W (1.11 ) 05/23/2017
 task: ffff8a293e4bcd80 task.stack: ffffa698027a8000
 RIP: 0010:tb_drom_read+0x383/0x890 [thunderbolt]
 RSP: 0018:ffffa698027ab990 EFLAGS: 00010246
 RAX: 0000000000000000 RBX: ffff8a2940af7800 RCX: 0000000000000000
 RDX: ffff8a2940ebb400 RSI: 0000000000000000 RDI: ffffa698027ab9a0
 RBP: ffffa698027ab9d0 R08: 0000000000000001 R09: 0000000000000002
 R10: ffff8a2940ebb5b0 R11: 0000000000000000 R12: ffff8a293bfa968c
 R13: 000000000000002c R14: 0000000000000056 R15: 0000000000000056
 FS:  00007f0a945a38c0(0000) GS:ffff8a2961580000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00000000000002ec CR3: 000000043e785000 CR4: 00000000003606e0
 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
 Call Trace:
  tb_switch_add+0x9d/0x730 [thunderbolt]
  ? tb_switch_alloc+0x3cd/0x4d0 [thunderbolt]
  icm_start+0x5a/0xa0 [thunderbolt]
  tb_domain_add+0xc3/0xf0 [thunderbolt]
  nhi_probe+0x19e/0x310 [thunderbolt]
  local_pci_probe+0x42/0xa0
  pci_device_probe+0x18d/0x1a0
  driver_probe_device+0x2ff/0x450
  __driver_attach+0xa4/0xe0
  ? driver_probe_device+0x450/0x450
  bus_for_each_dev+0x6e/0xb0
  driver_attach+0x1e/0x20
  bus_add_driver+0x1d0/0x270
  ? 0xffffffffc0bbb000
  driver_register+0x60/0xe0
  ? 0xffffffffc0bbb000
  __pci_register_driver+0x4c/0x50
  nhi_init+0x28/0x1000 [thunderbolt]
  do_one_initcall+0x50/0x190
  ? __vunmap+0x81/0xb0
  ? _cond_resched+0x1a/0x50
  ? kmem_cache_alloc_trace+0x15f/0x1c0
  ? do_init_module+0x27/0x1e9
  do_init_module+0x5f/0x1e9
  load_module+0x24e7/0x2a60
  ? vfs_read+0x115/0x130
  SYSC_finit_module+0xfc/0x120
  ? SYSC_finit_module+0xfc/0x120
  SyS_finit_module+0xe/0x10
  do_syscall_64+0x67/0x170
  entry_SYSCALL64_slow_path+0x25/0x25

Fix this by making sure we only enumerate DROM port entries the hardware
actually has.

Reported-by: Christian Kellner <ckellner@redhat.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
---
Changes from v1:
 - Fix typo in commit message
 - Use dev_info_once() instead and change the message not to include
   superfluous port numbers
 - Added Reviewed-by Lukas

 drivers/thunderbolt/eeprom.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
index 308b6e17c88a..fe2f00ceafc5 100644
--- a/drivers/thunderbolt/eeprom.c
+++ b/drivers/thunderbolt/eeprom.c
@@ -333,6 +333,15 @@ static int tb_drom_parse_entry_port(struct tb_switch *sw,
 	int res;
 	enum tb_port_type type;
 
+	/*
+	 * Some DROMs list more ports than the controller actually has
+	 * so we skip those but allow the parser to continue.
+	 */
+	if (header->index > sw->config.max_port_number) {
+		dev_info_once(&sw->dev, "ignoring unnecessary extra entries in DROM\n");
+		return 0;
+	}
+
 	port = &sw->ports[header->index];
 	port->disabled = header->port_disabled;
 	if (port->disabled)
-- 
2.13.2

             reply	other threads:[~2017-07-25 14:42 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-25 14:41 Mika Westerberg [this message]
2017-07-25 16:05 ` [PATCH v2] thunderbolt: Do not enumerate more ports from DROM than the controller has Christian Kellner

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=20170725144158.23806-1-mika.westerberg@linux.intel.com \
    --to=mika.westerberg@linux.intel.com \
    --cc=andreas.noever@gmail.com \
    --cc=ckellner@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=michael.jamet@intel.com \
    --cc=yehezkel.bernat@intel.com \
    /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.