Re: [PATCH mm-hotfixes] mm/vma: do not try to unmap a VMA if mmap_prepare() invoked from mmap()

[Pedro Falcato <pfalcato@suse.de>]

On Tue, Apr 21, 2026 at 11:21:50AM +0100, Lorenzo Stoakes wrote:
> The mmap_prepare hook functionality includes the ability to invoke
> mmap_prepare() from the mmap() hook of existing 'stacked' drivers, that is
> ones which are capable of calling the mmap hooks of other drivers/file
> systems (e.g. overlayfs, shm).
> 
> As part of the mmap_prepare action functionality, we deal with errors by
> unmapping the VMA should one arise. This works in the usual mmap_prepare
> case, as we invoke this action at the last moment, when the VMA is
> established in the maple tree.
> 
> However, the mmap() hook passes a not-fully-established VMA pointer to the
> caller (which is the motivation behind the mmap_prepare() work), which is
> detached.
> 
> So attempting to unmap a VMA in this state will be problematic, with the
> most obvious symptom being a warning in vma_mark_detached(), because the
> VMA is already detached.
> 
> It's also unncessary - the mmap() handler will clean up the VMA on error.
> 
> So to fix this issue, this patch propagates whether or not an mmap action
> is being completed via the compatibility layer or directly.
> 
> If the former, then we do not attempt VMA cleanup, if the latter, then we
> do.
> 
> This patch also updates the userland VMA tests to reflect the change.
> 
> Fixes: ac0a3fc9c07d ("mm: add ability to take further action in vm_area_desc")
> Cc: <stable@vger.kernel.org>
> Reported-by: syzbot+db390288d141a1dccf96@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/69e69734.050a0220.24bfd3.0027.GAE@google.com/
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>

How about something like the following:

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index a308e2c23b82..c29165de6d5c 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -868,6 +868,12 @@ struct mmap_action {
         * completely set up.
         */
        bool hide_from_rmap_until_complete :1;
+
+       /*
+        * Set if this mmap_action is part of compatibility with ->mmap().
+        * Internal flag.
+        */
+       bool compat_mmap :1;
 };
 
 /*
diff --git a/mm/util.c b/mm/util.c
index 232c3930a662..5134f879566d 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -1229,6 +1229,7 @@ int __compat_vma_mmap(struct vm_area_desc *desc,
        err = mmap_action_prepare(desc);
        if (err)
                return err;
+       desc->action.compat_mmap = 1;
        /* Update the VMA from the descriptor. */
        compat_set_vma_from_desc(vma, desc);
        /* Complete any specified mmap actions. */
@@ -1400,7 +1401,11 @@ static int mmap_action_finish(struct vm_area_struct *vma,
 
        /* do_munmap() might take rmap lock, so release if held. */
        maybe_rmap_unlock_action(vma, action);
-       if (!err)
+       /*
+        * If this is invoked from the compatibility layer, post-mmap() hook
+        * logic will handle cleanup for us.
+        */
+       if (!err || action->compat_mmap)
                return 0;
 
        /*


We have plenty of free bits in mmap_action and this is a little nicer than
passing is_compat bools down the callchain.

(that comment over compat_mmap is really... vague and bad, but I couldn't
think of something else)

-- 
Pedro