Re: [Openipmi-developer] [PATCH 0/1] Fix race in ipmi timer cleanup

From: Corey Minyard
Date: Sat Sep 14 2019 - 21:09:23 EST


>
> >
> > {disable,enable}_si_irq() themselves are racy:
> >
> > static inline bool disable_si_irq(struct smi_info *smi_info)
> > {
> > if ((smi_info->io.irq) && (!smi_info->interrupt_disabled)) {
> > smi_info->interrupt_disabled = true;
> >
> > Basically interrupt_disabled need to be atomic here to have any value,
> > unless you ensure to have a spin lock around every access to it.
>
> It needs to be atomic, yes, but I think just adding the spinlock like
> I suggested will work. You are right, the check for timer_running is
> not necessary here, and I'm fine with removing it, but there are other
> issues with interrupt_disabled (as you said) and with memory ordering
> in the timer case. So even if you remove the timer running check, the
> lock is still required here.

It turns out you were right, all that really needs to be done is the
del_timer_sync(). I've added your patch to my queue.

Sorry for the trouble.

Thanks,

-corey