From: Alan Stern [mailto:stern@xxxxxxxxxxxxxxxxxxx] > Sent: 14 December 2015 17:59 > On Mon, 14 Dec 2015, David Laight wrote: > > > From: Alan Stern > > > Sent: 14 December 2015 15:28 > > ... > > > I thought of something else, a more serious problem. According to the > > > man page for munmap(2), closing the file descriptor does not unmap the > > > region. Therefore we need to explicitly make sure that the usbcore > > > module cannot be unloaded while any memory mappings exist, by calling > > > try_module_get(THIS_MODULE) when a mapping is created and > > > module_put(THIS_MODULE) when a mapping is removed. > > > > Are you sure this is a problem? > > I haven't actually tried to provoke a crash, if that's what you're > asking. Apart from that, yes, I'm sure this is a problem. There are certainly potential issues with both remap_pfn_range() and vm_iomap_memory(). > > It is more likely that either: > > 1) the mmap() holds a reference to the physical pages - so they don't > > get freed even though the driver frees its reference to the memory. > > The problem has nothing to do with the pages being freed. > > > 2) when the driver frees the memory the user-space page tables are > > invalidated - so the next access generates EFAULT. > > I'm not sure what you're getting at here. The page tables are > invalidated when the user program calls munmap (or exits), and the > driver frees the memory when that happens. > > > Not that I've done the experiment (or tried to read the code). > > > > If you need to monitor the mapping, you need to allow for partial unmaps. > > That's not the problem either -- although I don't know how we should > handle partial unmaps. Can we disallow them? > > The problem is that the user can map the memory, then close the file > descriptor, then rmmod the usbcore module, then unmap the memory > region. When the unmap occurs, the memory subsystem will try to > dereference the usbdev_vm_ops structure -- which has been unloaded > along with the rest of usbcore. This will cause an oops. I was thinking that some side effect of dma_free_coherent() would find the user mapping for the pages, force unmap them and invalidate the user PTE (although the address range would have to remain reserved). Maybe that is wishful thinking. Similarly for iounmap(). > In theory we could prevent this problem by unmapping the memory when > the file descriptor is closed. But doing so would violate the > guarantee in the munmap(2) documentation. That doesn't help. The mapping is inherited by fork() but you only see the last close(). So the close() need not be in the context of the process that has the pages mapped. David -- 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
