From: Christophe JAILLET <christophe.jaillet@wanadoo.fr> To: nsaenz@kernel.org, jim2101024@gmail.com, f.fainelli@gmail.com, bcm-kernel-feedback-list@broadcom.com, lorenzo.pieralisi@arm.com, robh@kernel.org, kw@linux.com, bhelgaas@google.com Cc: linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, Christophe JAILLET <christophe.jaillet@wanadoo.fr> Subject: [PATCH] PCI: brcmstb: Declare a bitmap as a bitmap, not as a plain 'unsigned long' Date: Sun, 7 Nov 2021 09:32:58 +0100 [thread overview] Message-ID: <e6d9da2112aab2939d1507b90962d07bfd735b4c.1636273671.git.christophe.jaillet@wanadoo.fr> (raw) The 'used' field of 'struct brcm_msi' is used as a bitmap. So it should be declared as so (i.e. unsigned long *). This fixes an harmless Coverity warning about array vs singleton usage. This bitmap can be BRCM_INT_PCI_MSI_LEGACY_NR or BRCM_INT_PCI_MSI_NR long. So, while at it, document it, should it help someone in the future. Addresses-Coverity: "Out-of-bounds access (ARRAY_VS_SINGLETON)" Suggested-by: Krzysztof Wilczynski <kw@linux.com> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- The BUILD_BUG_ON is surely a bit to much of paranoia :) I'm also not really pleased about the layout of the DECLARE_BITMAP. This looks odd, but I couldn't find something nicer :( --- drivers/pci/controller/pcie-brcmstb.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c index 1fc7bd49a7ad..15d394ac7478 100644 --- a/drivers/pci/controller/pcie-brcmstb.c +++ b/drivers/pci/controller/pcie-brcmstb.c @@ -266,8 +266,9 @@ struct brcm_msi { struct mutex lock; /* guards the alloc/free operations */ u64 target_addr; int irq; - /* used indicates which MSI interrupts have been alloc'd */ - unsigned long used; + /* Used indicates which MSI interrupts have been alloc'd. 'nr' bellow is + the real size of the bitmap. It depends on the chip. */ + DECLARE_BITMAP (used, BRCM_INT_PCI_MSI_NR); bool legacy; /* Some chips have MSIs in bits [31..24] of a shared register. */ int legacy_shift; @@ -534,7 +535,7 @@ static int brcm_msi_alloc(struct brcm_msi *msi) int hwirq; mutex_lock(&msi->lock); - hwirq = bitmap_find_free_region(&msi->used, msi->nr, 0); + hwirq = bitmap_find_free_region(msi->used, msi->nr, 0); mutex_unlock(&msi->lock); return hwirq; @@ -543,7 +544,7 @@ static int brcm_msi_alloc(struct brcm_msi *msi) static void brcm_msi_free(struct brcm_msi *msi, unsigned long hwirq) { mutex_lock(&msi->lock); - bitmap_release_region(&msi->used, hwirq, 0); + bitmap_release_region(msi->used, hwirq, 0); mutex_unlock(&msi->lock); } @@ -661,6 +662,12 @@ static int brcm_pcie_enable_msi(struct brcm_pcie *pcie) msi->irq = irq; msi->legacy = pcie->hw_rev < BRCM_PCIE_HW_REV_33; + /* + * Sanity check to make sure that the 'used' bitmap in struct brcm_msi + * is large enough. + */ + BUILD_BUG_ON(BRCM_INT_PCI_MSI_LEGACY_NR > BRCM_INT_PCI_MSI_NR); + if (msi->legacy) { msi->intr_base = msi->base + PCIE_INTR2_CPU_BASE; msi->nr = BRCM_INT_PCI_MSI_LEGACY_NR; -- 2.30.2
WARNING: multiple messages have this Message-ID (diff)
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr> To: nsaenz@kernel.org, jim2101024@gmail.com, f.fainelli@gmail.com, bcm-kernel-feedback-list@broadcom.com, lorenzo.pieralisi@arm.com, robh@kernel.org, kw@linux.com, bhelgaas@google.com Cc: linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, Christophe JAILLET <christophe.jaillet@wanadoo.fr> Subject: [PATCH] PCI: brcmstb: Declare a bitmap as a bitmap, not as a plain 'unsigned long' Date: Sun, 7 Nov 2021 09:32:58 +0100 [thread overview] Message-ID: <e6d9da2112aab2939d1507b90962d07bfd735b4c.1636273671.git.christophe.jaillet@wanadoo.fr> (raw) The 'used' field of 'struct brcm_msi' is used as a bitmap. So it should be declared as so (i.e. unsigned long *). This fixes an harmless Coverity warning about array vs singleton usage. This bitmap can be BRCM_INT_PCI_MSI_LEGACY_NR or BRCM_INT_PCI_MSI_NR long. So, while at it, document it, should it help someone in the future. Addresses-Coverity: "Out-of-bounds access (ARRAY_VS_SINGLETON)" Suggested-by: Krzysztof Wilczynski <kw@linux.com> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- The BUILD_BUG_ON is surely a bit to much of paranoia :) I'm also not really pleased about the layout of the DECLARE_BITMAP. This looks odd, but I couldn't find something nicer :( --- drivers/pci/controller/pcie-brcmstb.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c index 1fc7bd49a7ad..15d394ac7478 100644 --- a/drivers/pci/controller/pcie-brcmstb.c +++ b/drivers/pci/controller/pcie-brcmstb.c @@ -266,8 +266,9 @@ struct brcm_msi { struct mutex lock; /* guards the alloc/free operations */ u64 target_addr; int irq; - /* used indicates which MSI interrupts have been alloc'd */ - unsigned long used; + /* Used indicates which MSI interrupts have been alloc'd. 'nr' bellow is + the real size of the bitmap. It depends on the chip. */ + DECLARE_BITMAP (used, BRCM_INT_PCI_MSI_NR); bool legacy; /* Some chips have MSIs in bits [31..24] of a shared register. */ int legacy_shift; @@ -534,7 +535,7 @@ static int brcm_msi_alloc(struct brcm_msi *msi) int hwirq; mutex_lock(&msi->lock); - hwirq = bitmap_find_free_region(&msi->used, msi->nr, 0); + hwirq = bitmap_find_free_region(msi->used, msi->nr, 0); mutex_unlock(&msi->lock); return hwirq; @@ -543,7 +544,7 @@ static int brcm_msi_alloc(struct brcm_msi *msi) static void brcm_msi_free(struct brcm_msi *msi, unsigned long hwirq) { mutex_lock(&msi->lock); - bitmap_release_region(&msi->used, hwirq, 0); + bitmap_release_region(msi->used, hwirq, 0); mutex_unlock(&msi->lock); } @@ -661,6 +662,12 @@ static int brcm_pcie_enable_msi(struct brcm_pcie *pcie) msi->irq = irq; msi->legacy = pcie->hw_rev < BRCM_PCIE_HW_REV_33; + /* + * Sanity check to make sure that the 'used' bitmap in struct brcm_msi + * is large enough. + */ + BUILD_BUG_ON(BRCM_INT_PCI_MSI_LEGACY_NR > BRCM_INT_PCI_MSI_NR); + if (msi->legacy) { msi->intr_base = msi->base + PCIE_INTR2_CPU_BASE; msi->nr = BRCM_INT_PCI_MSI_LEGACY_NR; -- 2.30.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next reply other threads:[~2021-11-07 8:33 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-11-07 8:32 Christophe JAILLET [this message] 2021-11-07 8:32 ` [PATCH] PCI: brcmstb: Declare a bitmap as a bitmap, not as a plain 'unsigned long' Christophe JAILLET 2021-11-08 1:34 ` Krzysztof Wilczyński 2021-11-08 1:34 ` Krzysztof Wilczyński 2021-11-08 16:28 ` Florian Fainelli 2021-11-08 16:28 ` Florian Fainelli 2021-11-08 19:51 ` Christophe JAILLET 2021-11-08 19:51 ` Christophe JAILLET 2021-11-08 23:30 ` Krzysztof Wilczyński 2021-11-08 23:30 ` Krzysztof Wilczyński 2021-11-08 23:45 ` Florian Fainelli 2021-11-08 23:45 ` Florian Fainelli 2021-11-08 23:55 ` Krzysztof Wilczyński 2021-11-08 23:55 ` Krzysztof Wilczyński 2021-11-08 23:44 ` Florian Fainelli 2021-11-08 23:44 ` Florian Fainelli 2021-11-29 13:11 ` Lorenzo Pieralisi 2021-11-29 13:11 ` Lorenzo Pieralisi 2021-11-30 17:04 ` Bjorn Helgaas 2021-11-30 17:04 ` Bjorn Helgaas 2021-12-03 11:02 ` Dan Carpenter 2021-12-03 11:02 ` Dan Carpenter
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=e6d9da2112aab2939d1507b90962d07bfd735b4c.1636273671.git.christophe.jaillet@wanadoo.fr \ --to=christophe.jaillet@wanadoo.fr \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=bhelgaas@google.com \ --cc=f.fainelli@gmail.com \ --cc=jim2101024@gmail.com \ --cc=kernel-janitors@vger.kernel.org \ --cc=kw@linux.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=linux-rpi-kernel@lists.infradead.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=nsaenz@kernel.org \ --cc=robh@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: linkBe 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.