Bug 272585 - calling mprotect in an mmap-ed stack can affect non-target pages
Summary: calling mprotect in an mmap-ed stack can affect non-target pages
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Konstantin Belousov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-07-19 00:40 UTC by John F. Carr
Modified: 2024-10-17 22:01 UTC (History)
3 users (show)

See Also:


Attachments
mmap-ed stack with an inaccessible gap (968 bytes, text/plain)
2023-07-19 00:40 UTC, John F. Carr
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description John F. Carr 2023-07-19 00:40:13 UTC
Created attachment 243475 [details]
mmap-ed stack with an inaccessible gap

If I call mmap with MAP_GROWSDOWN|MAP_STACK and use mprotect to mark the 32nd page from the top inaccessible (PROT_NONE), lower addresses in the region also become inaccessible.  This is an odd thing to do, I agree, but it happened in a real program.

Put another way: I call mmap with stack hints and get pages 0-255.  Before touching the memory I call mprotect(PROT_NONE) on page 224.  Now pages 1-223 are also inaccessible.

The target of mprotect has to be 32 pages down, not 31 or 33, at least with my system's configuration.  Perhaps 32 pages is the initial mapped size of a stack region and when the stack grows it clones the page attributes of the lowest address.

I attached an example that crashes reading an address that should be valid.  It works on Mac OS (without the unsupported MAP_GROWSDOWN|MAP_STACK flags) and Linux.

(This is simplified from a larger program that allocates a highly aligned 1 MB stack by mapping a larger region and using mprotect to install guard pages. It is not the "right" way to do it when MAP_ALIGNED is available.  The program was written first on Linux which doesn't have that option.)

Reproduced on 13.2-STABLE/amd64, 13.2-STABLE/aarch64, 14-CURRENT/amd64.
Comment 1 Konstantin Belousov freebsd_committer freebsd_triage 2023-07-19 11:14:07 UTC
https://reviews.freebsd.org/D41089
Comment 2 commit-hook freebsd_committer freebsd_triage 2023-07-20 14:12:54 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=21e45c30c35c9aa732073f725924caf581c93460

commit 21e45c30c35c9aa732073f725924caf581c93460
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2023-07-19 11:05:32 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2023-07-20 14:11:42 +0000

    mmap(MAP_STACK): on stack grow, use original protection

    If mprotect(2) changed protection in the bottom of the currently grown
    stack region, currently the changed protection would be used for the
    stack grow on next fault.  This is arguably unexpected.

    Store the original protection for the entry at mmap(2) time in the
    offset member of the gap vm_map_entry, and use it for protection of the
    grown stack region.

    PR:     272585
    Reported by:    John F. Carr <jfc@mit.edu>
    Reviewed by:    alc, markj
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D41089

 sys/vm/vm_map.c | 24 ++++++++++++++++--------
 sys/vm/vm_map.h |  4 ++++
 2 files changed, 20 insertions(+), 8 deletions(-)
Comment 3 commit-hook freebsd_committer freebsd_triage 2023-08-25 01:09:03 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=6c79559bf0c641a7b06966fa419935ca50dddedf

commit 6c79559bf0c641a7b06966fa419935ca50dddedf
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2023-07-19 11:05:32 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2023-08-25 01:06:43 +0000

    mmap(MAP_STACK): on stack grow, use original protection

    PR:     272585

    (cherry picked from commit 21e45c30c35c9aa732073f725924caf581c93460)

 sys/vm/vm_map.c | 24 ++++++++++++++++--------
 sys/vm/vm_map.h |  4 ++++
 2 files changed, 20 insertions(+), 8 deletions(-)
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2024-01-10 04:48:43 UTC
^Triage: assign to committer that resolved and MFCed.