Does vm_operations_struct require a .owner field?

Does vm_operations_struct require a .owner field?

[Alan Stern]

Hello:

Question: The vm_operations_struct structure contains lots of callback
pointers.  Is there any mechanism to prevent the callback routines and
the structure itself being unloaded from memory (if they are built into
modules) while the relevant VMAs are still in use?

Consider a simple example: A user program calls mmap(2) on a device
file.  Later on, the file is closed and the device driver's module is
unloaded.  But until munmap(2) is called or the user program exits, the
memory mapping and the corresponding VMA will remain in existence.  
(The man page for munmap specifically says "closing the file descriptor
does not unmap the region".)  Thus when the user program does do an
munmap(), the callback to vma->vm_ops->close will reference nonexistent
memory and cause an oops.

Normally this sort of thing is prevented by try_module_get(...->owner).  
But vm_operations_struct doesn't include a .owner field.

Am I missing something here?

Alan Stern

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Does vm_operations_struct require a .owner field?

[Kirill A. Shutemov]

On Tue, Jan 05, 2016 at 11:27:45AM -0500, Alan Stern wrote:
> Hello:
> 
> Question: The vm_operations_struct structure contains lots of callback
> pointers.  Is there any mechanism to prevent the callback routines and
> the structure itself being unloaded from memory (if they are built into
> modules) while the relevant VMAs are still in use?
> 
> Consider a simple example: A user program calls mmap(2) on a device
> file.  Later on, the file is closed and the device driver's module is
> unloaded.  But until munmap(2) is called or the user program exits, the
> memory mapping and the corresponding VMA will remain in existence.  
> (The man page for munmap specifically says "closing the file descriptor
> does not unmap the region".)  Thus when the user program does do an
> munmap(), the callback to vma->vm_ops->close will reference nonexistent
> memory and cause an oops.
> 
> Normally this sort of thing is prevented by try_module_get(...->owner).  
> But vm_operations_struct doesn't include a .owner field.
> 
> Am I missing something here?

mmap(2) takes reference of the file, therefore the file is not closed from
kernel POV until vma is gone and you cannot unload relevant module.
See get_file() in mmap_region().

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Does vm_operations_struct require a .owner field?

[Alan Stern]

On Tue, 5 Jan 2016, Kirill A. Shutemov wrote:

> On Tue, Jan 05, 2016 at 11:27:45AM -0500, Alan Stern wrote:
> > Hello:
> > 
> > Question: The vm_operations_struct structure contains lots of callback
> > pointers.  Is there any mechanism to prevent the callback routines and
> > the structure itself being unloaded from memory (if they are built into
> > modules) while the relevant VMAs are still in use?
> > 
> > Consider a simple example: A user program calls mmap(2) on a device
> > file.  Later on, the file is closed and the device driver's module is
> > unloaded.  But until munmap(2) is called or the user program exits, the
> > memory mapping and the corresponding VMA will remain in existence.  
> > (The man page for munmap specifically says "closing the file descriptor
> > does not unmap the region".)  Thus when the user program does do an
> > munmap(), the callback to vma->vm_ops->close will reference nonexistent
> > memory and cause an oops.
> > 
> > Normally this sort of thing is prevented by try_module_get(...->owner).  
> > But vm_operations_struct doesn't include a .owner field.
> > 
> > Am I missing something here?
> 
> mmap(2) takes reference of the file, therefore the file is not closed from
> kernel POV until vma is gone and you cannot unload relevant module.
> See get_file() in mmap_region().

Thank you.  So it looks like I was worried about nothing.

Steinar, you can remove the try_module_get/module_put lines from your
patch.  Also, the list_del() and comment in usbdev_release() aren't 
needed -- at that point we know the memory_list has to be empty since 
there can't be any outstanding URBs or VMA references.  If you take 
those things out then the patch should be ready for merging.

Alan Stern

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Does vm_operations_struct require a .owner field?

[Steinar H. Gunderson]

On Tue, Jan 05, 2016 at 04:31:09PM -0500, Alan Stern wrote:
> Thank you.  So it looks like I was worried about nothing.
> 
> Steinar, you can remove the try_module_get/module_put lines from your
> patch.  Also, the list_del() and comment in usbdev_release() aren't 
> needed -- at that point we know the memory_list has to be empty since 
> there can't be any outstanding URBs or VMA references.  If you take 
> those things out then the patch should be ready for merging.

Good, thanks. Did so, compiled, testing it still works, sending :-)

/* Steinar */
-- 
Software Engineer, Google Switzerland

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>