linux-kernel - Re: Possible 6.5 regression: Huge values for "commited memory"

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-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wh29JJSVGyJM7ubxOs51-Nxp6YnmU9Bw1gdOk3rrQ_0mg@mail.gmail.com>
Date:   Sat, 16 Sep 2023 12:31:42 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Bagas Sanjaya <bagasdotme@...il.com>,
        Michael Labiuk <michael.labiuk@...tuozzo.com>,
        Christoph Biedl <linux-kernel.bfrz@...chmal.in-ulm.de>
Cc:     Linux PARISC <linux-parisc@...r.kernel.org>,
        Linux Memory Management List <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Regressions <regressions@...ts.linux.dev>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>
Subject: Re: Possible 6.5 regression: Huge values for "commited memory"

On Sat, 16 Sept 2023 at 04:43, Bagas Sanjaya <bagasdotme@...il.com> wrote:
>
> Thanks for the regression report. Michael had already bisected it [1], so
> telling regzbot:
>
> #regzbot ^introduced: 408579cd627a15
> #regzbot title: huge committed memory due to returning 0 on do_vmi_align_mmunmap() success
>
> [1]: https://lore.kernel.org/linux-parisc/30f16b4f-a2fa-fc42-fe6e-abad01c3f794@virtuozzo.com/

Funky. That commit isn't actually supposed to change anything, and the
only locking change was because it incorrectly ended up doing the
unlock a bit too early (before it did a validate_mm() - fixed in
commit b5641a5d8b8b ("mm: don't do validate_mm() unnecessarily and
without mmap locking").

HOWEVER.

Now that I look at it again, I note this change in move_vma().

-       if (do_vmi_munmap(&vmi, mm, old_addr, old_len, uf_unmap, false) < 0) {
+       if (!do_vmi_munmap(&vmi, mm, old_addr, old_len, uf_unmap, false)) {

and I think that is wrong.

The return value that changed was the old "return 1 if successful
_and_ lock downgraded".

Now it does "lock is always released on success if requested". So the
special "1" return went away, but the failure case didn't change.

So that change to "move_vma()" seems to be bogus. It used to do "if
failed". Now it does "if success".

Does the attached patch fix the problem?

Liam - or am I just crazy? That return value check change really looks
bogus to me, but it looks *so* bogus that it makes me think I'm
missing something.

                       Linus

View attachment "patch.diff" of type "text/x-patch" (610 bytes)