From: Dan Carpenter <dan.carpenter@oracle.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Joao Pinto <Joao.Pinto@synopsys.com>,
Jingoo Han <jingoohan1@gmail.com>,
Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [patch] PCI: dwc: uninitialized variable in dw_handle_msi_irq()
Date: Tue, 7 Mar 2017 22:32:07 +0300 [thread overview]
Message-ID: <20170307192754.GC4120@mwanda> (raw)
In-Reply-To: <20170307190955.GE21358@bhelgaas-glaptop.roam.corp.google.com>
On Tue, Mar 07, 2017 at 01:09:55PM -0600, Bjorn Helgaas wrote:
> On Wed, Feb 22, 2017 at 03:08:07PM -0800, Joao Pinto wrote:
> > Hi Dan,
> >
> > Às 3:26 PM de 2/17/2017, Dan Carpenter escreveu:
> > > 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 <dan.carpenter@oracle.com>
> > > ---
> > > Static analysis. Not tested.
> >
> > What you are statiting makes perfect sense, since the register is indeed 32 bits
> > and can have undesirable behavior in 64-bit systems for example.
> > We have more examples like this for MSI related operations in pcie-designware.
> > Could you please change them as well just?
> >
> > For example, the irq variable declaration is also not consistent as you can see
> > in these examples:
> >
> > static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
> >
> > static int dw_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
> > irq_hw_number_t hwirq)
> >
> > static void dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
> >
> > static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
>
> Where are we with this? It sounds like there's a real problem here,
> and Dan's original patch fixes one case of it. But if there are other
> similar cases, we should fix them all at once.
>
> Since this doesn't sound like an urgent bug fix (I don't see user
> problem reports), I guess I'll wait for an updated patch?
Oh... Hm. I misread. I thought that Joao was going to send a patch.
Looking at it more closely now, I think my patch is sufficient. Perhaps
I have misunderstood something but I don't see any other bugs here
beyond the one I fixed.
regards,
dan carpenter
next prev parent reply other threads:[~2017-03-07 19:32 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-17 23:26 [patch] PCI: dwc: uninitialized variable in dw_handle_msi_irq() Dan Carpenter
2017-02-18 12:08 ` walter harms
2017-02-22 20:20 ` Bjorn Helgaas
2017-02-22 23:08 ` Joao Pinto
2017-03-07 19:09 ` Bjorn Helgaas
2017-03-07 19:32 ` Dan Carpenter [this message]
2017-03-16 19:44 ` Bjorn Helgaas
2017-03-17 8:26 ` walter harms
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=20170307192754.GC4120@mwanda \
--to=dan.carpenter@oracle.com \
--cc=Joao.Pinto@synopsys.com \
--cc=bhelgaas@google.com \
--cc=helgaas@kernel.org \
--cc=jingoohan1@gmail.com \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-pci@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).