On 12-11-19, 09:45, Logan Gunthorpe wrote: > > > On 2019-11-11 10:56 p.m., Vinod Koul wrote: > > On 11-11-19, 09:50, Logan Gunthorpe wrote: > >> > >> > >> On 2019-11-09 10:18 a.m., Vinod Koul wrote: > >>> Hi Logan, > >>> > >>> Sorry for delay in reply! > >>> > >>> On 22-10-19, 15:46, Logan Gunthorpe wrote: > >>>> dma_chan_to_owner() dereferences the driver from the struct device to > >>>> obtain the owner and call module_[get|put](). However, if the backing > >>>> device is unbound before the dma_device is unregistered, the driver > >>>> will be cleared and this will cause a NULL pointer dereference. > >>> > >>> Have you been able to repro this? If so how..? > >>> > >>> The expectation is that the driver shall unregister before removed. > >> > >> Yes, with my new driver, if I do a PCI unbind (which unregisters) while > >> the DMA engine is in use, it panics. The point is the underlying driver > >> can go away before the channel is removed. > > > > and in your driver remove you do not unregister? When unbind is invoked > > the driver remove is invoked by core and you should unregister whatever > > you have registered in your probe! > > > > Said that, if someone is using the dmaengine at that point of time, it > > is not a nice thing to do and can cause issues, but on idle it should > > just work! > > But that's the problem. We can't expect our users to be "nice" and not > unbind when the driver is in use. Killing the kernel if the user > unexpectedly unbinds is not acceptable. And that is why we review the code and ensure this does not happen and behaviour is as expected > >> I suspect this is less of an issue for most devices as they wouldn't > >> normally be unbound while in use (for example there's really no reason > >> to ever unbind IOAT seeing it's built into the system). Though, the fact > >> is, the user could unbind these devices at anytime and we don't want to > >> panic if they do. > > > > There are many drivers which do modules so yes I am expecting unbind and > > even a bind following that to work > > Except they will panic if they unbind while in use, so that's a > questionable definition of "work". dmaengine core has module reference so while they are being used they won't be removed (unless I complete misread the driver core behaviour) -- ~Vinod