drivers/infiniband/hw/hfi1/chip.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
handle_receive_interrupt_napi_sp() running inside interrupt handler
could introduce inverse lock ordering between &dd->irq_src_lock
and &dd->uctxt_lock, if read_mod_write() is preempted by the isr.
[CPU0] | [CPU1]
hfi1_ipoib_dev_open() |
--> hfi1_netdev_enable_queues() |
--> enable_queues(rx) |
--> hfi1_rcvctrl() |
--> set_intr_bits() |
--> read_mod_write() |
--> spin_lock(&dd->irq_src_lock) |
| hfi1_poll()
| --> poll_next()
| --> spin_lock_irq(&dd->uctxt_lock)
|
| --> hfi1_rcvctrl()
| --> set_intr_bits()
| --> read_mod_write()
| --> spin_lock(&dd->irq_src_lock)
<interrupt> |
--> handle_receive_interrupt_napi_sp() |
--> set_all_fastpath() |
--> hfi1_rcd_get_by_index() |
--> spin_lock_irqsave(&dd->uctxt_lock) |
This flaw was found by an experimental static analysis tool I am
developing for irq-related deadlock.
To prevent the potential deadlock, the patch use spin_lock_irqsave()
on &dd->irq_src_lock inside read_mod_write() to prevent the possible
deadlock scenario.
Signed-off-by: Chengfeng Ye <dg573847474@gmail.com>
---
drivers/infiniband/hw/hfi1/chip.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c
index 0814291a0412..9b542f7c6c11 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -13185,15 +13185,16 @@ static void read_mod_write(struct hfi1_devdata *dd, u16 src, u64 bits,
{
u64 reg;
u16 idx = src / BITS_PER_REGISTER;
+ unsigned long flags;
- spin_lock(&dd->irq_src_lock);
+ spin_lock_irqsave(&dd->irq_src_lock, flags);
reg = read_csr(dd, CCE_INT_MASK + (8 * idx));
if (set)
reg |= bits;
else
reg &= ~bits;
write_csr(dd, CCE_INT_MASK + (8 * idx), reg);
- spin_unlock(&dd->irq_src_lock);
+ spin_unlock_irqrestore(&dd->irq_src_lock, flags);
}
/**
--
2.17.1
On Tue, 26 Sep 2023 10:11:16 +0000, Chengfeng Ye wrote:
> handle_receive_interrupt_napi_sp() running inside interrupt handler
> could introduce inverse lock ordering between &dd->irq_src_lock
> and &dd->uctxt_lock, if read_mod_write() is preempted by the isr.
>
> [CPU0] | [CPU1]
> hfi1_ipoib_dev_open() |
> --> hfi1_netdev_enable_queues() |
> --> enable_queues(rx) |
> --> hfi1_rcvctrl() |
> --> set_intr_bits() |
> --> read_mod_write() |
> --> spin_lock(&dd->irq_src_lock) |
> | hfi1_poll()
> | --> poll_next()
> | --> spin_lock_irq(&dd->uctxt_lock)
> |
> | --> hfi1_rcvctrl()
> | --> set_intr_bits()
> | --> read_mod_write()
> | --> spin_lock(&dd->irq_src_lock)
> <interrupt> |
> --> handle_receive_interrupt_napi_sp() |
> --> set_all_fastpath() |
> --> hfi1_rcd_get_by_index() |
> --> spin_lock_irqsave(&dd->uctxt_lock) |
>
> [...]
Applied, thanks!
[1/1] IB/hfi1: Fix potential deadlock on &irq_src_lock and &dd->uctxt_lock
https://git.kernel.org/rdma/rdma/c/2f19c4b8395ccb
Best regards,
--
Leon Romanovsky <leon@kernel.org>
On Tue, Sep 26, 2023 at 10:11:16AM +0000, Chengfeng Ye wrote: > handle_receive_interrupt_napi_sp() running inside interrupt handler > could introduce inverse lock ordering between &dd->irq_src_lock > and &dd->uctxt_lock, if read_mod_write() is preempted by the isr. > > [CPU0] | [CPU1] > hfi1_ipoib_dev_open() | > --> hfi1_netdev_enable_queues() | > --> enable_queues(rx) | > --> hfi1_rcvctrl() | > --> set_intr_bits() | > --> read_mod_write() | > --> spin_lock(&dd->irq_src_lock) | > | hfi1_poll() > | --> poll_next() > | --> spin_lock_irq(&dd->uctxt_lock) > | > | --> hfi1_rcvctrl() > | --> set_intr_bits() > | --> read_mod_write() > | --> spin_lock(&dd->irq_src_lock) > <interrupt> | > --> handle_receive_interrupt_napi_sp() | > --> set_all_fastpath() | > --> hfi1_rcd_get_by_index() | > --> spin_lock_irqsave(&dd->uctxt_lock) | > > This flaw was found by an experimental static analysis tool I am > developing for irq-related deadlock. > > To prevent the potential deadlock, the patch use spin_lock_irqsave() > on &dd->irq_src_lock inside read_mod_write() to prevent the possible > deadlock scenario. > > Signed-off-by: Chengfeng Ye <dg573847474@gmail.com> > --- > drivers/infiniband/hw/hfi1/chip.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Dennis? This needs your ack/nack Thanks, Jason
On 10/23/23 12:01 PM, Jason Gunthorpe wrote: > On Tue, Sep 26, 2023 at 10:11:16AM +0000, Chengfeng Ye wrote: >> handle_receive_interrupt_napi_sp() running inside interrupt handler >> could introduce inverse lock ordering between &dd->irq_src_lock >> and &dd->uctxt_lock, if read_mod_write() is preempted by the isr. >> >> [CPU0] | [CPU1] >> hfi1_ipoib_dev_open() | >> --> hfi1_netdev_enable_queues() | >> --> enable_queues(rx) | >> --> hfi1_rcvctrl() | >> --> set_intr_bits() | >> --> read_mod_write() | >> --> spin_lock(&dd->irq_src_lock) | >> | hfi1_poll() >> | --> poll_next() >> | --> spin_lock_irq(&dd->uctxt_lock) >> | >> | --> hfi1_rcvctrl() >> | --> set_intr_bits() >> | --> read_mod_write() >> | --> spin_lock(&dd->irq_src_lock) >> <interrupt> | >> --> handle_receive_interrupt_napi_sp() | >> --> set_all_fastpath() | >> --> hfi1_rcd_get_by_index() | >> --> spin_lock_irqsave(&dd->uctxt_lock) | >> >> This flaw was found by an experimental static analysis tool I am >> developing for irq-related deadlock. >> >> To prevent the potential deadlock, the patch use spin_lock_irqsave() >> on &dd->irq_src_lock inside read_mod_write() to prevent the possible >> deadlock scenario. >> >> Signed-off-by: Chengfeng Ye <dg573847474@gmail.com> >> --- >> drivers/infiniband/hw/hfi1/chip.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) > > Dennis? This needs your ack/nack Looks like we need to disable the interrupt. Sorry for the delay. Acked-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
© 2016 - 2025 Red Hat, Inc.