Messages in this thread | | | Subject | Re: [PATCH v3 3/3] uio: fix crash after the device is unregistered | From | Xiubo Li <> | Date | Sat, 7 Jul 2018 09:28:45 +0800 |
| |
On 2018/7/7 2:23, Mike Christie wrote: > On 07/05/2018 09:57 PM, xiubli@redhat.com wrote: >> static irqreturn_t uio_interrupt(int irq, void *dev_id) >> { >> struct uio_device *idev = (struct uio_device *)dev_id; >> - irqreturn_t ret = idev->info->handler(irq, idev->info); >> + irqreturn_t ret; >> + >> + mutex_lock(&idev->info_lock); >> + if (!idev->info) { >> + ret = IRQ_NONE; >> + goto out; >> + } >> >> + ret = idev->info->handler(irq, idev->info); >> if (ret == IRQ_HANDLED) >> uio_event_notify(idev->info); >> >> +out: >> + mutex_unlock(&idev->info_lock); >> return ret; >> } > > Do you need the interrupt related changes in this patch and the first > one? Actually, the NULL checking is not a must, we can remove this. But the lock/unlock is needed. > When we do uio_unregister_device -> free_irq does free_irq return > when there are no longer running interrupt handlers that we requested? > > If that is not the case then I think we can hit a similar bug. We do: > > __uio_register_device -> device_register -> device's refcount goes to > zero so we do -> uio_device_release -> kfree(idev) > > and if it is possible the interrupt handler could still run after > free_irq then we would end up doing: > > uio_interrupt -> mutex_lock(&idev->info_lock) -> idev access freed memory.
I think this shouldn't happen. Because the free_irq function does not return until any executing interrupts for this IRQ have completed.
Thanks, BRs
| |