On Mon, 18 Dec 2023 21:25:16 +0000 Chris Diamand <chris.diamand@xxxxxxxxxxxx> wrote: > > +void panthor_fw_unplug(struct panthor_device *ptdev) > > +{ > > + struct panthor_fw_section *section; > > + > > + cancel_delayed_work_sync(&ptdev->fw->watchdog.ping_work); > > + > > + /* Make sure the IRQ handler can be called after that > > point. */ > > + if (ptdev->fw->irq.irq) > > + panthor_job_irq_suspend(&ptdev->fw->irq); > > + > > + panthor_fw_stop(ptdev); > > + > > + if (ptdev->fw->vm) > > + panthor_vm_idle(ptdev->fw->vm); > > + > > + list_for_each_entry(section, &ptdev->fw->sections, node) > > + panthor_kernel_bo_destroy(panthor_fw_vm(ptdev), > > section->mem); > > Here's where we destroy the potentially invalid section->mem. > > Note that the issues are twofold: > Firstly, section->mem could be an error pointer as mentioned earlier. > Secondly, panthor_kernel_bo_destroy() doesn't actually check for > error values or even NULL. > > It would probably be worth adding NULL checks to > panthor_kernel_bo_destroy() and panthor_kernel_bo_vunmap() to protect > against this. I ended up implementing your suggestion, because it simplifies all call-sites in panthor_sched.c too. The new version defer the IS_ERR_OR_NULL() check to panthor_kernel_bo_destroy().