On Tue, 5 Jan 2016, David Laight wrote: > From: Steinar H. Gunderson [mailto:se...@google.com] > > 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. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
