linux-kernel - Re: [PATCH hotfix 6.12 0/8] fix error handling in mmap

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: <78f09435-71c6-49d2-8d31-05456b9b0313@lucifer.local>
Date: Wed, 23 Oct 2024 09:45:47 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Jann Horn <jannh@...gle.com>, 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 0/8] fix error handling in mmap_region() and
 refactor

On Wed, Oct 23, 2024 at 10:37:04AM +0200, Vlastimil Babka wrote:
> On 10/22/24 22:40, Lorenzo Stoakes wrote:
> > The mmap_region() function is somewhat terrifying, with spaghetti-like
> > control flow and numerous means by which issues can arise and incomplete
> > state, memory leaks and other unpleasantness can occur.
> >
> > A large amount of the complexity arises from trying to handle errors late
> > in the process of mapping a VMA, which forms the basis of recently observed
> > issues with resource leaks and observable inconsistent state.
> >
> > This series goes to great lengths to simplify how mmap_region() works and
> > to avoid unwinding errors late on in the process of setting up the VMA for
> > the new mapping, and equally avoids such operations occurring while the VMA
> > is in an inconsistent state.
> >
> > The first four patches are intended for backporting to correct the
> > possibility of people encountering corrupted state while invoking mmap()
> > which is otherwise at risk of happening.
> >
> > After this we go further, refactoring the code, placing it in mm/vma.c in
> > order to make it eventually userland testable, and significantly
> > simplifying the logic to avoid this issue arising in future.
>
> Are the latter 4 patches thus also intended as hotfix for 6.12, or was it
> just due to git applying the same PATCH prefix to the whole series, and the
> real intention is 6.13?

They are intended as they are in the same vein, so one could argue we
should hotfix that and be done with it... but at the same time I realise
it's a bit of a stretch so I'm happy to split the last four patches for
6.13.

The issue is expressing that the the last 4 have a dependency on the first
4 and this not getting lost in the mix...

To be clear though, the first 4 do very much _have_ to be hotfixes and ones
which are backported to stable.

Andrew - we can do whatever makes sense to you here.

Thanks!

>
> > Lorenzo Stoakes (8):
> >   mm: avoid unsafe VMA hook invocation when error arises on mmap hook
> >   mm: unconditionally close VMAs on error
> >   mm: refactor map_deny_write_exec()
> >   mm: resolve faulty mmap_region() error path behaviour
> >   tools: testing: add additional vma_internal.h stubs
> >   mm: insolate mmap internal logic to mm/vma.c
> >   mm: refactor __mmap_region()
> >   mm: do not attempt second merge for file-backed VMAs
> >
> >  include/linux/mman.h             |  21 +-
> >  mm/internal.h                    |  44 ++++
> >  mm/mmap.c                        | 262 ++------------------
> >  mm/mprotect.c                    |   2 +-
> >  mm/nommu.c                       |   7 +-
> >  mm/vma.c                         | 403 ++++++++++++++++++++++++++++++-
> >  mm/vma.h                         | 101 +-------
> >  mm/vma_internal.h                |   5 +
> >  tools/testing/vma/vma_internal.h | 106 +++++++-
> >  9 files changed, 591 insertions(+), 360 deletions(-)
> >
> > --
> > 2.47.0
>