linux-kernel - [PATCH v2] isci: correct erroneous for_each_isci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20140121113354.19036.70624.stgit@gklab-154-244.igk.intel.com>
Date:	Tue, 21 Jan 2014 12:33:54 +0100
From:	Lukasz Dorau <lukasz.dorau@...el.com>
To:	JBottomley@...allels.com
Cc:	dan.j.williams@...el.com, dave.jiang@...el.com,
	maciej.patelczyk@...el.com, linux-scsi@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH v2] isci: correct erroneous for_each_isci_host macro

In the first place, the loop 'for' in the macro 'for_each_isci_host'
(drivers/scsi/isci/host.h:314) is incorrect, because it accesses
the 3rd element of 2 element array. After the 2nd iteration it executes
the instruction:
        ihost = to_pci_info(pdev)->hosts[2]
(while the size of the 'hosts' array equals 2) and reads an
out of range element.

In the second place, this loop is incorrectly optimized by GCC v4.8
(see http://marc.info/?l=linux-kernel&m=138998871911336&w=2).
As a result, on platforms with two SCU controllers,
the loop is executed more times than it can be (for i=0,1 and 2).
It causes kernel panic during entering the S3 state
and the following oops after 'rmmod isci':

BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<ffffffff8131360b>] __list_add+0x1b/0xc0
Oops: 0000 [#1] SMP
RIP: 0010:[<ffffffff8131360b>]  [<ffffffff8131360b>] __list_add+0x1b/0xc0
Call Trace:
  [<ffffffff81661b84>] __mutex_lock_slowpath+0x114/0x1b0
  [<ffffffff81661c3f>] mutex_lock+0x1f/0x30
  [<ffffffffa03e97cb>] sas_disable_events+0x1b/0x50 [libsas]
  [<ffffffffa03e9818>] sas_unregister_ha+0x18/0x60 [libsas]
  [<ffffffffa040316e>] isci_unregister+0x1e/0x40 [isci]
  [<ffffffffa0403efd>] isci_pci_remove+0x5d/0x100 [isci]
  [<ffffffff813391cb>] pci_device_remove+0x3b/0xb0
  [<ffffffff813fbf7f>] __device_release_driver+0x7f/0xf0
  [<ffffffff813fc8f8>] driver_detach+0xa8/0xb0
  [<ffffffff813fbb8b>] bus_remove_driver+0x9b/0x120
  [<ffffffff813fcf2c>] driver_unregister+0x2c/0x50
  [<ffffffff813381f3>] pci_unregister_driver+0x23/0x80
  [<ffffffffa04152f8>] isci_exit+0x10/0x1e [isci]
  [<ffffffff810d199b>] SyS_delete_module+0x16b/0x2d0
  [<ffffffff81012a21>] ? do_notify_resume+0x61/0xa0
  [<ffffffff8166ce29>] system_call_fastpath+0x16/0x1b

The loop has been corrected.
This patch fixes kernel panic during entering the S3 state
and the above oops.

Signed-off-by: Lukasz Dorau <lukasz.dorau@...el.com>
Reviewed-by: Maciej Patelczyk <maciej.patelczyk@...el.com>
Reviewed-by: Dan Williams <dan.j.williams@...el.com>
Tested-by: Lukasz Dorau <lukasz.dorau@...el.com>
Cc: Dave Jiang <dave.jiang@...el.com>
Cc: <stable@...r.kernel.org>
---
v2:
- updated changelog, removed initialization of ihost,
  ARRAY_SIZE macro replaced with SCI_MAX_CONTROLLERS
---
 drivers/scsi/isci/host.h |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/isci/host.h b/drivers/scsi/isci/host.h
index 4911310..22a9bb1 100644
--- a/drivers/scsi/isci/host.h
+++ b/drivers/scsi/isci/host.h
@@ -311,9 +311,8 @@ static inline struct Scsi_Host *to_shost(struct isci_host *ihost)
 }

 #define for_each_isci_host(id, ihost, pdev) \
-	for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
-	     id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
-	     ihost = to_pci_info(pdev)->hosts[++id])
+	for (id = 0; id < SCI_MAX_CONTROLLERS && \
+	     (ihost = to_pci_info(pdev)->hosts[id]); id++)

 static inline void wait_for_start(struct isci_host *ihost)
 {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/