linux-kernel - Re: [PATCH hotfix 6.12 1/8] mm: avoid unsafe VMA hook invocation when error arises on mmap hook

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <854e4713-2579-43c4-8d57-04ed059f3250@lucifer.local>
Date: Wed, 23 Oct 2024 17:56:11 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Jann Horn <jannh@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Vlastimil Babka <vbabka@...e.cz>, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Xu <peterx@...hat.com>
Subject: Re: [PATCH hotfix 6.12 1/8] mm: avoid unsafe VMA hook invocation
 when error arises on mmap hook

On Tue, Oct 22, 2024 at 11:14:58PM +0200, Jann Horn wrote:
> On Tue, Oct 22, 2024 at 10:41 PM Lorenzo Stoakes
> <lorenzo.stoakes@...cle.com> wrote:
> > After an attempted mmap() fails, we are no longer in a situation where we
> > can safely interact with VMA hooks. This is currently not enforced, meaning
> > that we need complicated handling to ensure we do not incorrectly call
> > these hooks.
> >
> > We can avoid the whole issue by treating the VMA as suspect the moment that
> > the file->f_ops->mmap() function reports an error by replacing whatever VMA
> > operations were installed with a dummy empty set of VMA operations.
> >
> > We do so through a new helper function internal to mm - mmap_file() - which
> > is both more logically named than the existing call_mmap() function and
> > correctly isolates handling of the vm_op reassignment to mm.
> >
> > All the existing invocations of call_mmap() outside of mm are ultimately
> > nested within the call_mmap() from mm, which we now replace.
> >
> > It is therefore safe to leave call_mmap() in place as a convenience
> > function (and to avoid churn). The invokers are:
> >
> >      ovl_file_operations -> mmap -> ovl_mmap() -> backing_file_mmap()
> >     coda_file_operations -> mmap -> coda_file_mmap()
> >      shm_file_operations -> shm_mmap()
> > shm_file_operations_huge -> shm_mmap()
> >             dma_buf_fops -> dma_buf_mmap_internal -> i915_dmabuf_ops
> >                             -> i915_gem_dmabuf_mmap()
> >
> > None of these callers interact with vm_ops or mappings in a problematic way
> > on error, quickly exiting out.
> >
> > Reported-by: Jann Horn <jannh@...gle.com>
> > Fixes: deb0f6562884 ("mm/mmap: undo ->mmap() when arch_validate_flags() fails")
>
> (I guess the "Fixes" tag here is a little unconventional in that it
> doesn't actually point at the commit introducing the issue that this
> commit describes, but it does mark to where the fix should be
> backported, so I guess it makes sense and I don't have any better
> suggestion.)

Yeah it's unfortunate but I think the only thing we can do here.

>
> > Cc: stable <stable@...nel.org>
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
>
> Reviewed-by: Jann Horn <jannh@...gle.com>

Thanks!