On Tue, Jan 5, 2016 at 4:57 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > On Tue, 5 Jan 2016, David Laight wrote: > >> From: Steinar H. Gunderson [mailto:sesse@xxxxxxxxxx] >> > Sent: 26 November 2015 00:19 >> >> There is still a problem with this code, not sure how to fix it. >> >> ... >> > +static void dec_usb_memory_use_count(struct usb_memory *usbm, int *count) >> > +{ >> > + struct usb_dev_state *ps = usbm->ps; >> > + unsigned long flags; >> > + >> > + spin_lock_irqsave(&ps->lock, flags); >> > + --*count; >> > + if (usbm->urb_use_count == 0 && usbm->vma_use_count == 0) { >> ... >> > + module_put(THIS_MODULE); >> > + } else { >> > + spin_unlock_irqrestore(&ps->lock, flags); >> > + } >> > +} >> ... >> > +static void usbdev_vm_close(struct vm_area_struct *vma) >> > +{ >> > + struct usb_memory *usbm = vma->vm_private_data; >> > + >> > + dec_usb_memory_use_count(usbm, &usbm->vma_use_count); >> > +} >> >> If the last reference to the module is for an mmap request >> (which is the only reason to reference count the module here) >> then the module_put() in dec_usb_memory_use_count()) returns >> back into freed memory. > > Ooh, you're right. > >> There isn't much the module can do about it either apart >> from using kthread_run() to call module_put_and_exit() >> and even that is somewhat racy (a sleep in the kthread >> is probably enough). >> >> The only real solution is for mmap() itself to take the >> reference on the module. > > I agree; the vm_operations_struct structure ought to have a .owner > member. But I don't feel like going through and changing all of them. > > How do other drivers handle this problem? Perhaps they don't face it > because they don't use DMA mapping to implement mmap. A normal MMIO > sort of page mapping doesn't cause difficulties if you leave it in > place after the device is unregistered. > > Or maybe I just don't understand the problem very well and the core > kernel handles all of this for us. I'll try asking. > > kthread_run plus sleep and module_put_and_exit might turn out to be the > way to go. If anyone has a better suggestion, I'd like to hear it. > you might check the video4linux implementation, they do DMA mapping of videoframes but not sure if they run into the same problem somehow. Markus -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
