linux-kernel - Re: brk() in v6.1-rc1 can expand file mappings, seemingly without taking file locks

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: <20221205145005.ku75npr3dsz3fqgo@revolver>
Date:   Mon, 5 Dec 2022 14:50:12 +0000
From:   Liam Howlett <liam.howlett@...cle.com>
To:     Jann Horn <jannh@...gle.com>
CC:     Linux-MM <linux-mm@...ck.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        kernel list <linux-kernel@...r.kernel.org>,
        Jason Donenfeld <Jason@...c4.com>, Yu Zhao <yuzhao@...gle.com>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        SeongJae Park <sj@...nel.org>, Vlastimil Babka <vbabka@...e.cz>
Subject: Re: brk() in v6.1-rc1 can expand file mappings, seemingly without
 taking file locks

* Jann Horn <jannh@...gle.com> [221202 13:54]:
> As of commit ca57f02295f, brk() can expand ordinary file mappings (but
> not file mappings with weird flags), and I think it does it with
> insufficient locks. I think brk() probably needs some extra checks to
> make sure it's operating on a brk-like VMA (which means it should at
> least be anonymous, and perhaps pass the full can_vma_merge_after()
> check so that we're not creating unnecessary special cases?).


Thanks.  This is probably caused by commit 2e7ce7d354f2: "mm/mmap:
change do_brk_flags() to expand existing VMA and add do_brk_munmap()"

Specifically the checks around expanding the VMA. 

> user@vm:~/brk_stretch$ cat brk_file.c

Thanks for the testcase.  I have a fix that I'm testing, but it's worth
noting that the brk call will succeed - except a new VMA will be
created.  Is this what you expect?

...
> 
> The codepaths that are intended to expand file VMAs do stuff like
> i_mmap_lock_write() and vma_interval_tree_remove(), which
> do_brk_flags() doesn't seem to do (because it was never intended to
> operate on file VMAs?).

I don't think the locks were there before and I think you are correct
that it wasn't intended to operate on file VMAs.  I made sure all the
ltp tests around brk pass with my changes but this seems insufficient.