public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Tom Lane <tgl@sss.pgh.pa.us>,
	Daniel Axtens <dja@axtens.net>,
	Michael Ellerman <mpe@ellerman.id.au>
Subject: [PATCH 4.4 28/33] powerpc: Allow 4224 bytes of stack expansion for the signal frame
Date: Mon, 24 Aug 2020 10:31:24 +0200	[thread overview]
Message-ID: <20200824082347.948251307@linuxfoundation.org> (raw)
In-Reply-To: <20200824082346.498653578@linuxfoundation.org>

From: Michael Ellerman <mpe@ellerman.id.au>

commit 63dee5df43a31f3844efabc58972f0a206ca4534 upstream.

We have powerpc specific logic in our page fault handling to decide if
an access to an unmapped address below the stack pointer should expand
the stack VMA.

The code was originally added in 2004 "ported from 2.4". The rough
logic is that the stack is allowed to grow to 1MB with no extra
checking. Over 1MB the access must be within 2048 bytes of the stack
pointer, or be from a user instruction that updates the stack pointer.

The 2048 byte allowance below the stack pointer is there to cover the
288 byte "red zone" as well as the "about 1.5kB" needed by the signal
delivery code.

Unfortunately since then the signal frame has expanded, and is now
4224 bytes on 64-bit kernels with transactional memory enabled. This
means if a process has consumed more than 1MB of stack, and its stack
pointer lies less than 4224 bytes from the next page boundary, signal
delivery will fault when trying to expand the stack and the process
will see a SEGV.

The total size of the signal frame is the size of struct rt_sigframe
(which includes the red zone) plus __SIGNAL_FRAMESIZE (128 bytes on
64-bit).

The 2048 byte allowance was correct until 2008 as the signal frame
was:

struct rt_sigframe {
        struct ucontext    uc;                           /*     0  1440 */
        /* --- cacheline 11 boundary (1408 bytes) was 32 bytes ago --- */
        long unsigned int          _unused[2];           /*  1440    16 */
        unsigned int               tramp[6];             /*  1456    24 */
        struct siginfo *           pinfo;                /*  1480     8 */
        void *                     puc;                  /*  1488     8 */
        struct siginfo     info;                         /*  1496   128 */
        /* --- cacheline 12 boundary (1536 bytes) was 88 bytes ago --- */
        char                       abigap[288];          /*  1624   288 */

        /* size: 1920, cachelines: 15, members: 7 */
        /* padding: 8 */
};

1920 + 128 = 2048

Then in commit ce48b2100785 ("powerpc: Add VSX context save/restore,
ptrace and signal support") (Jul 2008) the signal frame expanded to
2304 bytes:

struct rt_sigframe {
        struct ucontext    uc;                           /*     0  1696 */	<--
        /* --- cacheline 13 boundary (1664 bytes) was 32 bytes ago --- */
        long unsigned int          _unused[2];           /*  1696    16 */
        unsigned int               tramp[6];             /*  1712    24 */
        struct siginfo *           pinfo;                /*  1736     8 */
        void *                     puc;                  /*  1744     8 */
        struct siginfo     info;                         /*  1752   128 */
        /* --- cacheline 14 boundary (1792 bytes) was 88 bytes ago --- */
        char                       abigap[288];          /*  1880   288 */

        /* size: 2176, cachelines: 17, members: 7 */
        /* padding: 8 */
};

2176 + 128 = 2304

At this point we should have been exposed to the bug, though as far as
I know it was never reported. I no longer have a system old enough to
easily test on.

Then in 2010 commit 320b2b8de126 ("mm: keep a guard page below a
grow-down stack segment") caused our stack expansion code to never
trigger, as there was always a VMA found for a write up to PAGE_SIZE
below r1.

That meant the bug was hidden as we continued to expand the signal
frame in commit 2b0a576d15e0 ("powerpc: Add new transactional memory
state to the signal context") (Feb 2013):

struct rt_sigframe {
        struct ucontext    uc;                           /*     0  1696 */
        /* --- cacheline 13 boundary (1664 bytes) was 32 bytes ago --- */
        struct ucontext    uc_transact;                  /*  1696  1696 */	<--
        /* --- cacheline 26 boundary (3328 bytes) was 64 bytes ago --- */
        long unsigned int          _unused[2];           /*  3392    16 */
        unsigned int               tramp[6];             /*  3408    24 */
        struct siginfo *           pinfo;                /*  3432     8 */
        void *                     puc;                  /*  3440     8 */
        struct siginfo     info;                         /*  3448   128 */
        /* --- cacheline 27 boundary (3456 bytes) was 120 bytes ago --- */
        char                       abigap[288];          /*  3576   288 */

        /* size: 3872, cachelines: 31, members: 8 */
        /* padding: 8 */
        /* last cacheline: 32 bytes */
};

3872 + 128 = 4000

And commit 573ebfa6601f ("powerpc: Increase stack redzone for 64-bit
userspace to 512 bytes") (Feb 2014):

struct rt_sigframe {
        struct ucontext    uc;                           /*     0  1696 */
        /* --- cacheline 13 boundary (1664 bytes) was 32 bytes ago --- */
        struct ucontext    uc_transact;                  /*  1696  1696 */
        /* --- cacheline 26 boundary (3328 bytes) was 64 bytes ago --- */
        long unsigned int          _unused[2];           /*  3392    16 */
        unsigned int               tramp[6];             /*  3408    24 */
        struct siginfo *           pinfo;                /*  3432     8 */
        void *                     puc;                  /*  3440     8 */
        struct siginfo     info;                         /*  3448   128 */
        /* --- cacheline 27 boundary (3456 bytes) was 120 bytes ago --- */
        char                       abigap[512];          /*  3576   512 */	<--

        /* size: 4096, cachelines: 32, members: 8 */
        /* padding: 8 */
};

4096 + 128 = 4224

Then finally in 2017, commit 1be7107fbe18 ("mm: larger stack guard
gap, between vmas") exposed us to the existing bug, because it changed
the stack VMA to be the correct/real size, meaning our stack expansion
code is now triggered.

Fix it by increasing the allowance to 4224 bytes.

Hard-coding 4224 is obviously unsafe against future expansions of the
signal frame in the same way as the existing code. We can't easily use
sizeof() because the signal frame structure is not in a header. We
will either fix that, or rip out all the custom stack expansion
checking logic entirely.

Fixes: ce48b2100785 ("powerpc: Add VSX context save/restore, ptrace and signal support")
Cc: stable@vger.kernel.org # v2.6.27+
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Tested-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20200724092528.1578671-2-mpe@ellerman.id.au
Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/powerpc/mm/fault.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -192,6 +192,9 @@ static int mm_fault_error(struct pt_regs
 	return MM_FAULT_CONTINUE;
 }
 
+// This comes from 64-bit struct rt_sigframe + __SIGNAL_FRAMESIZE
+#define SIGFRAME_MAX_SIZE	(4096 + 128)
+
 /*
  * For 600- and 800-family processors, the error_code parameter is DSISR
  * for a data fault, SRR1 for an instruction fault. For 400-family processors
@@ -341,7 +344,7 @@ retry:
 	/*
 	 * N.B. The POWER/Open ABI allows programs to access up to
 	 * 288 bytes below the stack pointer.
-	 * The kernel signal delivery code writes up to about 1.5kB
+	 * The kernel signal delivery code writes up to about 4kB
 	 * below the stack pointer (r1) before decrementing it.
 	 * The exec code can write slightly over 640kB to the stack
 	 * before setting the user r1.  Thus we allow the stack to
@@ -365,7 +368,7 @@ retry:
 		 * between the last mapped region and the stack will
 		 * expand the stack rather than segfaulting.
 		 */
-		if (address + 2048 < uregs->gpr[1] && !store_update_sp)
+		if (address + SIGFRAME_MAX_SIZE < uregs->gpr[1] && !store_update_sp)
 			goto bad_area;
 	}
 	if (expand_stack(vma, address))



  parent reply	other threads:[~2020-08-24  8:50 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-24  8:30 [PATCH 4.4 00/33] 4.4.234-rc1 review Greg Kroah-Hartman
2020-08-24  8:30 ` [PATCH 4.4 01/33] drm/imx: imx-ldb: Disable both channels for split mode in enc->disable() Greg Kroah-Hartman
2020-08-24  8:30 ` [PATCH 4.4 02/33] perf probe: Fix memory leakage when the probe point is not found Greg Kroah-Hartman
2020-08-24  8:30 ` [PATCH 4.4 03/33] net/compat: Add missing sock updates for SCM_RIGHTS Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 04/33] watchdog: f71808e_wdt: indicate WDIOF_CARDRESET support in watchdog_info.options Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 05/33] watchdog: f71808e_wdt: remove use of wrong watchdog_info option Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 06/33] coredump: fix race condition between collapse_huge_page() and core dumping Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 07/33] khugepaged: khugepaged_test_exit() check mmget_still_valid() Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 08/33] khugepaged: adjust VM_BUG_ON_MM() in __khugepaged_enter() Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 09/33] btrfs: export helpers for subvolume name/id resolution Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 10/33] btrfs: dont show full path of bind mounts in subvol= Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 11/33] romfs: fix uninitialized memory leak in romfs_dev_read() Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 12/33] mm: include CMA pages in lowmem_reserve at boot Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 13/33] mm, page_alloc: fix core hung in free_pcppages_bulk() Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 14/33] ext4: clean up ext4_match() and callers Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 15/33] ext4: fix checking of directory entry validity for inline directories Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 16/33] media: budget-core: Improve exception handling in budget_register() Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 17/33] media: vpss: clean up resources in init Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 18/33] Input: psmouse - add a newline when printing proto by sysfs Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 19/33] m68knommu: fix overwriting of bits in ColdFire V3 cache control Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 20/33] xfs: fix inode quota reservation checks Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 21/33] jffs2: fix UAF problem Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 22/33] scsi: libfc: Free skb in fc_disc_gpn_id_resp() for valid cases Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 23/33] virtio_ring: Avoid loop when vq is broken in virtqueue_poll Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 24/33] xfs: Fix UBSAN null-ptr-deref in xfs_sysfs_init Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 25/33] alpha: fix annotation of io{read,write}{16,32}be() Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 26/33] ext4: fix potential negative array index in do_split() Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 27/33] ASoC: intel: Fix memleak in sst_media_open Greg Kroah-Hartman
2020-08-24  8:31 ` Greg Kroah-Hartman [this message]
2020-08-24  8:31 ` [PATCH 4.4 29/33] epoll: Keep a reference on files added to the check list Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 30/33] do_epoll_ctl(): clean the failure exits up a bit Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 31/33] mm/hugetlb: fix calculation of adjust_range_if_pmd_sharing_possible Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 32/33] xen: dont reschedule in preemption off sections Greg Kroah-Hartman
2020-08-24  8:31 ` [PATCH 4.4 33/33] omapfb: dss: Fix max fclk divider for omap36xx Greg Kroah-Hartman
2020-08-26  8:09 ` [PATCH 4.4 00/33] 4.4.234-rc1 review Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200824082347.948251307@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=dja@axtens.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=stable@vger.kernel.org \
    --cc=tgl@sss.pgh.pa.us \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox