Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752355AbaATSZw (ORCPT ); Mon, 20 Jan 2014 13:25:52 -0500 Received: from mail-qe0-f53.google.com ([209.85.128.53]:60810 "EHLO mail-qe0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750743AbaATSZr (ORCPT ); Mon, 20 Jan 2014 13:25:47 -0500 MIME-Version: 1.0 In-Reply-To: <20140120165450.15332.39118.stgit@gklab-154-244.igk.intel.com> References: <20140120165450.15332.39118.stgit@gklab-154-244.igk.intel.com> Date: Mon, 20 Jan 2014 10:25:46 -0800 X-Google-Sender-Auth: 40PpU6z0D_2yVpxoGsxRlc82TVw Message-ID: Subject: Re: [PATCH] isci: correct erroneous for_each_isci_host macro From: Dan Williams To: Lukasz Dorau Cc: "JBottomley@parallels.com" , "dave.jiang" , "Patelczyk, Maciej" , linux-scsi , "Baldysiak, Pawel" , Linux Kernel Mailing List Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 20, 2014 at 8:54 AM, Lukasz Dorau wrote: > 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, because of the following GCC v4.8 bug: > http://marc.info/?l=linux-kernel&m=138998871911336&w=2 > this loop is incorrectly compiled by GCC v4.8. > As a result, on platforms with two SCU controllers, > the loop at drivers/scsi/isci/init.c:717 is executed > more times than it can be (for i=0,1 and 2). > It causes the following oops after 'rmmod isci': > Surprised those bugs lasted this long... some comments below. > BUG: unable to handle kernel NULL pointer dereference at (null) > IP: [] __list_add+0x1b/0xc0 > Oops: 0000 [#1] SMP > RIP: 0010:[] [] __list_add+0x1b/0xc0 > Call Trace: > [] __mutex_lock_slowpath+0x114/0x1b0 > [] mutex_lock+0x1f/0x30 > [] sas_disable_events+0x1b/0x50 [libsas] > [] sas_unregister_ha+0x18/0x60 [libsas] > [] isci_unregister+0x1e/0x40 [isci] > [] isci_pci_remove+0x5d/0x100 [isci] > [] pci_device_remove+0x3b/0xb0 > [] __device_release_driver+0x7f/0xf0 > [] driver_detach+0xa8/0xb0 > [] bus_remove_driver+0x9b/0x120 > [] driver_unregister+0x2c/0x50 > [] pci_unregister_driver+0x23/0x80 > [] isci_exit+0x10/0x1e [isci] > [] SyS_delete_module+0x16b/0x2d0 > [] ? do_notify_resume+0x61/0xa0 > [] system_call_fastpath+0x16/0x1b > > The loop has been corrected. > This patch fixes the above oops. > > Signed-off-by: Lukasz Dorau > Reviewed-by: Maciej Patelczyk > Tested-by: Lukasz Dorau > Cc: Dan Williams > Cc: Dave Jiang > Cc: > --- > drivers/scsi/isci/host.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/isci/host.h b/drivers/scsi/isci/host.h > index 4911310..95adf31 100644 > --- a/drivers/scsi/isci/host.h > +++ b/drivers/scsi/isci/host.h > @@ -311,9 +311,9 @@ 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, ihost = to_pci_info(pdev)->hosts[0]; \ No need to initialize ihost here since we assign and check it later. > + (id < ARRAY_SIZE(to_pci_info(pdev)->hosts)) && \ Just use SCI_MAX_CONTROLLERS here. > + (ihost = to_pci_info(pdev)->hosts[id]); id++) -- Dan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/