From: Sean Anderson <sean.anderson@linux.dev>
To: "Lorenzo Pieralisi" <lpieralisi@kernel.org>,
"Krzysztof Wilczyński" <kw@linux.com>,
"Rob Herring" <robh@kernel.org>,
linux-pci@vger.kernel.org
Cc: Thippeswamy Havalige <thippeswamy.havalige@amd.com>,
linux-arm-kernel@lists.infradead.org,
Markus Elfring <Markus.Elfring@web.de>,
Dan Carpenter <dan.carpenter@linaro.org>,
linux-kernel@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
Michal Simek <michal.simek@amd.com>,
Sean Anderson <sean.anderson@linux.dev>,
stable@vger.kernel.org, Bharat Kumar Gogada <bharatku@xilinx.com>,
Bjorn Helgaas <helgaas@kernel.org>
Subject: [PATCH v4 2/7] PCI: xilinx-nwl: Fix off-by-one in IRQ handler
Date: Fri, 31 May 2024 12:13:32 -0400 [thread overview]
Message-ID: <20240531161337.864994-3-sean.anderson@linux.dev> (raw)
In-Reply-To: <20240531161337.864994-1-sean.anderson@linux.dev>
MSGF_LEG_MASK is laid out with INTA in bit 0, INTB in bit 1, INTC in bit
2, and INTD in bit 3. Hardware IRQ numbers start at 0, and we register
PCI_NUM_INTX irqs. So to enable INTA (aka hwirq 0) we should set bit 0.
Remove the subtraction of one.
This bug would cause legacy interrupts not to be delivered, as enabling
INTB would actually enable INTA, and enabling INTA wouldn't enable
anything at all. It is likely that this got overlooked for so long since
most PCIe hardware uses MSIs. This fixes the following UBSAN error:
UBSAN: shift-out-of-bounds in ../drivers/pci/controller/pcie-xilinx-nwl.c:389:11
shift exponent 18446744073709551615 is too large for 32-bit type 'int'
CPU: 1 PID: 61 Comm: kworker/u10:1 Not tainted 6.6.20+ #268
Hardware name: xlnx,zynqmp (DT)
Workqueue: events_unbound deferred_probe_work_func
Call trace:
dump_backtrace (arch/arm64/kernel/stacktrace.c:235)
show_stack (arch/arm64/kernel/stacktrace.c:242)
dump_stack_lvl (lib/dump_stack.c:107)
dump_stack (lib/dump_stack.c:114)
__ubsan_handle_shift_out_of_bounds (lib/ubsan.c:218 lib/ubsan.c:387)
nwl_unmask_leg_irq (drivers/pci/controller/pcie-xilinx-nwl.c:389 (discriminator 1))
irq_enable (kernel/irq/internals.h:234 kernel/irq/chip.c:170 kernel/irq/chip.c:439 kernel/irq/chip.c:432 kernel/irq/chip.c:345)
__irq_startup (kernel/irq/internals.h:239 kernel/irq/chip.c:180 kernel/irq/chip.c:250)
irq_startup (kernel/irq/chip.c:270)
__setup_irq (kernel/irq/manage.c:1800)
request_threaded_irq (kernel/irq/manage.c:2206)
pcie_pme_probe (include/linux/interrupt.h:168 drivers/pci/pcie/pme.c:348)
<snip>
Fixes: 9a181e1093af ("PCI: xilinx-nwl: Modify IRQ chip for legacy interrupts")
Cc: <stable@vger.kernel.org>
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---
Changes in v4:
- Explain likely effects of the off-by-one error
- Trim down UBSAN backtrace
Changes in v3:
- Expand commit message
drivers/pci/controller/pcie-xilinx-nwl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 0408f4d612b5..437927e3bcca 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -371,7 +371,7 @@ static void nwl_mask_intx_irq(struct irq_data *data)
u32 mask;
u32 val;
- mask = 1 << (data->hwirq - 1);
+ mask = 1 << data->hwirq;
raw_spin_lock_irqsave(&pcie->leg_mask_lock, flags);
val = nwl_bridge_readl(pcie, MSGF_LEG_MASK);
nwl_bridge_writel(pcie, (val & (~mask)), MSGF_LEG_MASK);
@@ -385,7 +385,7 @@ static void nwl_unmask_intx_irq(struct irq_data *data)
u32 mask;
u32 val;
- mask = 1 << (data->hwirq - 1);
+ mask = 1 << data->hwirq;
raw_spin_lock_irqsave(&pcie->leg_mask_lock, flags);
val = nwl_bridge_readl(pcie, MSGF_LEG_MASK);
nwl_bridge_writel(pcie, (val | mask), MSGF_LEG_MASK);
--
2.35.1.1320.gc452695387.dirty
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-05-31 16:14 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-31 16:13 [PATCH v4 0/7] PCI: xilinx-nwl: Add phy support Sean Anderson
2024-05-31 16:13 ` [PATCH v4 1/7] dt-bindings: pci: xilinx-nwl: Add phys property Sean Anderson
2024-05-31 16:13 ` Sean Anderson [this message]
2024-05-31 16:13 ` [PATCH v4 3/7] PCI: xilinx-nwl: Fix register misspelling Sean Anderson
2024-05-31 16:13 ` [PATCH v4 4/7] PCI: xilinx-nwl: Rate-limit misc interrupt messages Sean Anderson
2024-05-31 16:13 ` [PATCH v4 5/7] PCI: xilinx-nwl: Clean up clock on probe failure/removal Sean Anderson
2024-05-31 16:13 ` [PATCH v4 6/7] PCI: xilinx-nwl: Add phy support Sean Anderson
2024-06-01 13:10 ` Markus Elfring
2024-05-31 16:13 ` [PATCH v4 7/7] arm64: zynqmp: Add PCIe phys Sean Anderson
2024-06-03 8:17 ` Michal Simek
2024-08-09 19:43 ` [PATCH v4 0/7] PCI: xilinx-nwl: Add phy support Sean Anderson
2024-08-09 19:54 ` Bjorn Helgaas
2024-08-30 14:08 ` Michal Simek
2024-08-30 15:53 ` Bjorn Helgaas
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=20240531161337.864994-3-sean.anderson@linux.dev \
--to=sean.anderson@linux.dev \
--cc=Markus.Elfring@web.de \
--cc=bharatku@xilinx.com \
--cc=bhelgaas@google.com \
--cc=dan.carpenter@linaro.org \
--cc=helgaas@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=lpieralisi@kernel.org \
--cc=michal.simek@amd.com \
--cc=robh@kernel.org \
--cc=stable@vger.kernel.org \
--cc=thippeswamy.havalige@amd.com \
/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).