Messages in this thread Patch in this message | | | Date | Wed, 28 Jun 2023 06:47:29 -0500 | From | Corey Minyard <> | Subject | Re: [PATCH] ipmi: fix potential deadlock on &kcs_bmc->lock |
| |
Indeed, this looks like an issue.
Andrew, any opinions on this? The attached patch will work, the other option would be to disable interrupts when calling kcs_bmc_handle_event() in the timer handler. But then you have to worry about RT.
-corey
On Tue, Jun 27, 2023 at 03:24:49PM +0000, Chengfeng Ye wrote: > As kcs_bmc_handle_event() is executed inside both a timer and a hardirq, > it should disable irq before lock acquisition otherwise deadlock could > happen if the timmer is preemtped by the irq. > > Possible deadlock scenario: > aspeed_kcs_check_obe() (timer) > -> kcs_bmc_handle_event() > -> spin_lock(&kcs_bmc->lock) > <irq interruption> > -> aspeed_kcs_irq() > -> kcs_bmc_handle_event() > -> spin_lock(&kcs_bmc->lock) (deadlock here) > > This flaw was found using an experimental static analysis tool we are > developing for irq-related deadlock. > > The tentative patch fix the potential deadlock by spin_lock_irqsave() > > Signed-off-by: Chengfeng Ye <dg573847474@gmail.com> > --- > drivers/char/ipmi/kcs_bmc.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c > index 03d02a848f3a..8b1161d5194a 100644 > --- a/drivers/char/ipmi/kcs_bmc.c > +++ b/drivers/char/ipmi/kcs_bmc.c > @@ -56,12 +56,13 @@ irqreturn_t kcs_bmc_handle_event(struct kcs_bmc_device *kcs_bmc) > { > struct kcs_bmc_client *client; > irqreturn_t rc = IRQ_NONE; > + unsigned long flags; > > - spin_lock(&kcs_bmc->lock); > + spin_lock_irqsave(&kcs_bmc->lock, flags); > client = kcs_bmc->client; > if (client) > rc = client->ops->event(client); > - spin_unlock(&kcs_bmc->lock); > + spin_unlock_irqrestore(&kcs_bmc->lock, flags); > > return rc; > } > -- > 2.17.1 >
| |