mm: avoid deadlock when holding rmap on mmap_prepare error
Commitac0a3fc9c0("mm: add ability to take further action in vm_area_desc") added the ability for drivers to instruct mm to take actions after the .mmap_prepare callback is complete. To make life simpler and safer, this is done before the VMA/mmap write lock is dropped but when the VMA is completely established. So on error, we simply munmap() the VMA. As part of this implementation, unfortunately a horrible hack had to be implemented to support some questionable behaviour hugetlb relies upon - that is that the file rmap lock is held until the operation is complete. The implementation, for convenience, did this in mmap_action_finish() so both the VMA and mmap_prepare compatibility layer paths would have this correctly handled. However, it turns out there is a mistake here - the rmap lock cannot be held on munmap, as free_pgtables() -> unlink_file_vma_batch_add() -> unlink_file_vma_batch_process() takes the file rmap lock. We therefore currently have a deadlock issue that might arise. Resolve this by leaving it to callers to handle the unmap. The compatibility layer does not support this rmap behaviour, so we simply have it unmap on error after calling mmap_action_complete(). In the VMA implementation, we only perform the unmap after the rmap lock is dropped. This resolves the issue by ensuring the rmap lock is always dropped when the unmap occurs. Link: https://lkml.kernel.org/r/d44248be9da68258b07c2c59d4e73485ee0ca943.1774045440.git.ljs@kernel.org Fixes:ac0a3fc9c0("mm: add ability to take further action in vm_area_desc") Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org> Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Bodo Stroesser <bostroesser@gmail.com> Cc: Christian Brauner <brauner@kernel.org> Cc: Clemens Ladisch <clemens@ladisch.de> Cc: David Hildenbrand <david@kernel.org> Cc: David Howells <dhowells@redhat.com> Cc: Dexuan Cui <decui@microsoft.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Haiyang Zhang <haiyangz@microsoft.com> Cc: Jan Kara <jack@suse.cz> Cc: Jann Horn <jannh@google.com> Cc: Jonathan Corbet <corbet@lwn.net> Cc: K. Y. Srinivasan <kys@microsoft.com> Cc: Liam Howlett <liam.howlett@oracle.com> Cc: Long Li <longli@microsoft.com> Cc: Marc Dionne <marc.dionne@auristor.com> Cc: "Martin K. Petersen" <martin.petersen@oracle.com> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> Cc: Michal Hocko <mhocko@suse.com> Cc: Mike Rapoport <rppt@kernel.org> Cc: Miquel Raynal <miquel.raynal@bootlin.com> Cc: Pedro Falcato <pfalcato@suse.de> Cc: Richard Weinberger <richard@nod.at> Cc: Ryan Roberts <ryan.roberts@arm.com> Cc: Suren Baghdasaryan <surenb@google.com> Cc: Vignesh Raghavendra <vigneshr@ti.com> Cc: Vlastimil Babka (SUSE) <vbabka@kernel.org> Cc: Wei Liu <wei.liu@kernel.org> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
This commit is contained in:
parent
827e97cf4b
commit
f96e1d5f15
2 changed files with 17 additions and 8 deletions
12
mm/util.c
12
mm/util.c
|
|
@ -1215,7 +1215,13 @@ int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
|
|||
return err;
|
||||
|
||||
set_vma_from_desc(vma, &desc);
|
||||
return mmap_action_complete(vma, &desc.action);
|
||||
err = mmap_action_complete(vma, &desc.action);
|
||||
if (err) {
|
||||
const size_t len = vma_pages(vma) << PAGE_SHIFT;
|
||||
|
||||
do_munmap(current->mm, vma->vm_start, len, NULL);
|
||||
}
|
||||
return err;
|
||||
}
|
||||
EXPORT_SYMBOL(compat_vma_mmap);
|
||||
|
||||
|
|
@ -1316,10 +1322,6 @@ static int mmap_action_finish(struct vm_area_struct *vma,
|
|||
* invoked if we do NOT merge, so we only clean up the VMA we created.
|
||||
*/
|
||||
if (err) {
|
||||
const size_t len = vma_pages(vma) << PAGE_SHIFT;
|
||||
|
||||
do_munmap(current->mm, vma->vm_start, len, NULL);
|
||||
|
||||
if (action->error_hook) {
|
||||
/* We may want to filter the error. */
|
||||
err = action->error_hook(err);
|
||||
|
|
|
|||
13
mm/vma.c
13
mm/vma.c
|
|
@ -2735,9 +2735,9 @@ static int call_action_complete(struct mmap_state *map,
|
|||
struct mmap_action *action,
|
||||
struct vm_area_struct *vma)
|
||||
{
|
||||
int ret;
|
||||
int err;
|
||||
|
||||
ret = mmap_action_complete(vma, action);
|
||||
err = mmap_action_complete(vma, action);
|
||||
|
||||
/* If we held the file rmap we need to release it. */
|
||||
if (map->hold_file_rmap_lock) {
|
||||
|
|
@ -2745,7 +2745,14 @@ static int call_action_complete(struct mmap_state *map,
|
|||
|
||||
i_mmap_unlock_write(file->f_mapping);
|
||||
}
|
||||
return ret;
|
||||
|
||||
if (err) {
|
||||
const size_t len = vma_pages(vma) << PAGE_SHIFT;
|
||||
|
||||
do_munmap(current->mm, vma->vm_start, len, NULL);
|
||||
}
|
||||
|
||||
return err;
|
||||
}
|
||||
|
||||
static unsigned long __mmap_region(struct file *file, unsigned long addr,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue