From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0214B23EA8A for ; Tue, 21 Apr 2026 11:02:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776769376; cv=none; b=Cs5h4wzcvGcHK57HY11yfy8yhB4NErp4wLJwYXQz4rPg1cC/IDMThX7rn/pIUUDXx1C7rzAeT3zAPA37xNKMw/aFk9foesBI54VD+aGFxWpDCQfzbZRaTI284XiOojnS62aJPAALXylSRiqGTCsvguHCFlFaad0o9GCirA9PlI4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776769376; c=relaxed/simple; bh=Mit3d++puFieDkYFiFSR5MLYsiwR/pliP6/RpT1qDsE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=mdyKTUe+qvTjdxfpaJVhZyZTu/r3AGac4tnm/xaUlnkWTl18TWzJybMANa1bXeWe5+hDGk6Tb/YnLjzbJguopsfQR/l3lWrcNdC62g7u1/39Kt60skg/W6HnykUWvm2FlZNSX8A8L39TCcx6X7r6ZxpVKw4NJ2MWR+EQMvr0Jd0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=GFtayXn/; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="GFtayXn/" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2B4B0C2BCB0; Tue, 21 Apr 2026 11:02:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1776769375; bh=Mit3d++puFieDkYFiFSR5MLYsiwR/pliP6/RpT1qDsE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=GFtayXn/xyAWSKM3nxHS9I00rGB4QSlbKFd0XxmKa6XJxsSeaKsETqmsbCHcSYFwj 3GotQgqxxYRaw3HaVEKkXqX4YofTeRXNMEAS/fT5rmWDHZGprsL4gxz+Y4eu7lDIXS 0BOueZ5Xv4gjvzFlGKOxtZIYqevTCaQuCS054OuT9uwCrOd13Eet1pZ3hH9o02k5eH U33+QmMZfMQrIHpQlyHR6OXgCC+QSw9lSCcYh+2On7N9AYk54jGJ6/gOpjHCvjK/xS LTmhgKOaN3eZaJghcNHLP3j3UGHO/3hRVnRMiPfM+wwzEQ2pNXfu8g7kSi/08Stw+j Qo8zG6usKxQCg== Date: Tue, 21 Apr 2026 12:02:50 +0100 From: Lorenzo Stoakes To: Pedro Falcato Cc: Andrew Morton , David Hildenbrand , "Liam R . Howlett" , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , Jann Horn , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH mm-hotfixes] mm/vma: do not try to unmap a VMA if mmap_prepare() invoked from mmap() Message-ID: References: <20260421102150.189982-1-ljs@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Tue, Apr 21, 2026 at 11:52:23AM +0100, Pedro Falcato wrote: > 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: > > 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 > > How about something like the following: (Normally I'd love helper struct stuf but...) I was going to say in the commit message as to why I'm not doing that :) but thought maybe I shouldn't spell it out. But to spell it out - I don't want to do that, because it could be abused by drivers and I don't want to add any possibility of doing that, as it defeats the purpose of the change. And adding checks to make sure a driver didn't mess with it complicates everything. I'd rather live with the added param, it's temporary and can be removed once the mmap() hook is removed... > > 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 Cheers, Lorenzo