From patchwork Thu Mar 16 19:44:57 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 739960 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3vkf804FJHz9ryZ for ; Fri, 17 Mar 2017 06:45:04 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752487AbdCPTpD (ORCPT ); Thu, 16 Mar 2017 15:45:03 -0400 Received: from mail.kernel.org ([198.145.29.136]:43308 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752367AbdCPTpC (ORCPT ); Thu, 16 Mar 2017 15:45:02 -0400 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E4F9820465; Thu, 16 Mar 2017 19:44:59 +0000 (UTC) Received: from localhost (unknown [69.55.156.165]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 7DFAC202FE; Thu, 16 Mar 2017 19:44:58 +0000 (UTC) Date: Thu, 16 Mar 2017 14:44:57 -0500 From: Bjorn Helgaas To: Dan Carpenter Cc: Jingoo Han , Joao Pinto , Bjorn Helgaas , linux-pci@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [patch] PCI: dwc: uninitialized variable in dw_handle_msi_irq() Message-ID: <20170316194457.GB9739@bhelgaas-glaptop.roam.corp.google.com> References: <20170217232618.GC26717@mwanda> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170217232618.GC26717@mwanda> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Sat, Feb 18, 2017 at 02:26:18AM +0300, Dan Carpenter wrote: > The bug is that "val" is unsigned long but we only initialize 32 bits > of it. Then we test "if (val)" and that might be true not because we > set the bits but because some were never initialized. > > Fixes: f342d940ee0e ("PCI: exynos: Add support for MSI") > Signed-off-by: Dan Carpenter I applied this to pci/host-designware for v4.12. I also applied the patch below based on walter's suggestion: commit b67d3c69df8d6721f87bbc22a587914e0d4944a7 Author: Bjorn Helgaas Date: Thu Mar 16 14:34:59 2017 -0500 PCI: dwc: Unindent dw_handle_msi_irq() loop Use "continue" to skip rest of the loop when possible to save an indent level. No functional change intended. Suggested-by: walter harms Signed-off-by: Bjorn Helgaas > --- > Static analysis. Not tested. > > diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c > index af8f6e92e885..5bfc377b83e4 100644 > --- a/drivers/pci/dwc/pcie-designware.c > +++ b/drivers/pci/dwc/pcie-designware.c > @@ -257,17 +257,18 @@ static struct irq_chip dw_msi_irq_chip = { > /* MSI int handler */ > irqreturn_t dw_handle_msi_irq(struct pcie_port *pp) > { > - unsigned long val; > + u32 val; > int i, pos, irq; > irqreturn_t ret = IRQ_NONE; > > for (i = 0; i < MAX_MSI_CTRLS; i++) { > dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4, > - (u32 *)&val); > + &val); > if (val) { > ret = IRQ_HANDLED; > pos = 0; > - while ((pos = find_next_bit(&val, 32, pos)) != 32) { > + while ((pos = find_next_bit((unsigned long *)&val, 32, > + pos)) != 32) { > irq = irq_find_mapping(pp->irq_domain, > i * 32 + pos); > dw_pcie_wr_own_conf(pp, Acked-By: wharms@bfs.de diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c index 5ba334938b52..6cdd41f06dea 100644 --- a/drivers/pci/dwc/pcie-designware-host.c +++ b/drivers/pci/dwc/pcie-designware-host.c @@ -63,17 +63,17 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp) for (i = 0; i < MAX_MSI_CTRLS; i++) { dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4, (u32 *)&val); - if (val) { - ret = IRQ_HANDLED; - pos = 0; - while ((pos = find_next_bit(&val, 32, pos)) != 32) { - irq = irq_find_mapping(pp->irq_domain, - i * 32 + pos); - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + - i * 12, 4, 1 << pos); - generic_handle_irq(irq); - pos++; - } + if (!val) + continue; + + ret = IRQ_HANDLED; + pos = 0; + while ((pos = find_next_bit(&val, 32, pos)) != 32) { + irq = irq_find_mapping(pp->irq_domain, i * 32 + pos); + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, + 4, 1 << pos); + generic_handle_irq(irq); + pos++; } }