linux-kernel.vger.kernel.org archive mirror
 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, Eric Biggers <ebiggers@google.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Ingo Molnar <mingo@kernel.org>,
	Konstantin Khlebnikov <koct9i@gmail.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Michal Hocko <mhocko@suse.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH 4.12 10/27] mm, uprobes: fix multiple free of ->uprobes_state.xol_area
Date: Tue,  5 Sep 2017 09:11:26 +0200	[thread overview]
Message-ID: <20170905070923.654450153@linuxfoundation.org> (raw)
In-Reply-To: <20170905070923.265950493@linuxfoundation.org>

4.12-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Eric Biggers <ebiggers@google.com>

commit 355627f518978b5167256d27492fe0b343aaf2f2 upstream.

Commit 7c051267931a ("mm, fork: make dup_mmap wait for mmap_sem for
write killable") made it possible to kill a forking task while it is
waiting to acquire its ->mmap_sem for write, in dup_mmap().

However, it was overlooked that this introduced an new error path before
the new mm_struct's ->uprobes_state.xol_area has been set to NULL after
being copied from the old mm_struct by the memcpy in dup_mm().  For a
task that has previously hit a uprobe tracepoint, this resulted in the
'struct xol_area' being freed multiple times if the task was killed at
just the right time while forking.

Fix it by setting ->uprobes_state.xol_area to NULL in mm_init() rather
than in uprobe_dup_mmap().

With CONFIG_UPROBE_EVENTS=y, the bug can be reproduced by the same C
program given by commit 2b7e8665b4ff ("fork: fix incorrect fput of
->exe_file causing use-after-free"), provided that a uprobe tracepoint
has been set on the fork_thread() function.  For example:

    $ gcc reproducer.c -o reproducer -lpthread
    $ nm reproducer | grep fork_thread
    0000000000400719 t fork_thread
    $ echo "p $PWD/reproducer:0x719" > /sys/kernel/debug/tracing/uprobe_events
    $ echo 1 > /sys/kernel/debug/tracing/events/uprobes/enable
    $ ./reproducer

Here is the use-after-free reported by KASAN:

    BUG: KASAN: use-after-free in uprobe_clear_state+0x1c4/0x200
    Read of size 8 at addr ffff8800320a8b88 by task reproducer/198

    CPU: 1 PID: 198 Comm: reproducer Not tainted 4.13.0-rc7-00015-g36fde05f3fb5 #255
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-20170228_101828-anatol 04/01/2014
    Call Trace:
     dump_stack+0xdb/0x185
     print_address_description+0x7e/0x290
     kasan_report+0x23b/0x350
     __asan_report_load8_noabort+0x19/0x20
     uprobe_clear_state+0x1c4/0x200
     mmput+0xd6/0x360
     do_exit+0x740/0x1670
     do_group_exit+0x13f/0x380
     get_signal+0x597/0x17d0
     do_signal+0x99/0x1df0
     exit_to_usermode_loop+0x166/0x1e0
     syscall_return_slowpath+0x258/0x2c0
     entry_SYSCALL_64_fastpath+0xbc/0xbe

    ...

    Allocated by task 199:
     save_stack_trace+0x1b/0x20
     kasan_kmalloc+0xfc/0x180
     kmem_cache_alloc_trace+0xf3/0x330
     __create_xol_area+0x10f/0x780
     uprobe_notify_resume+0x1674/0x2210
     exit_to_usermode_loop+0x150/0x1e0
     prepare_exit_to_usermode+0x14b/0x180
     retint_user+0x8/0x20

    Freed by task 199:
     save_stack_trace+0x1b/0x20
     kasan_slab_free+0xa8/0x1a0
     kfree+0xba/0x210
     uprobe_clear_state+0x151/0x200
     mmput+0xd6/0x360
     copy_process.part.8+0x605f/0x65d0
     _do_fork+0x1a5/0xbd0
     SyS_clone+0x19/0x20
     do_syscall_64+0x22f/0x660
     return_from_SYSCALL_64+0x0/0x7a

Note: without KASAN, you may instead see a "Bad page state" message, or
simply a general protection fault.

Link: http://lkml.kernel.org/r/20170830033303.17927-1-ebiggers3@gmail.com
Fixes: 7c051267931a ("mm, fork: make dup_mmap wait for mmap_sem for write killable")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Reported-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Konstantin Khlebnikov <koct9i@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 kernel/events/uprobes.c |    2 --
 kernel/fork.c           |    8 ++++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1262,8 +1262,6 @@ void uprobe_end_dup_mmap(void)
 
 void uprobe_dup_mmap(struct mm_struct *oldmm, struct mm_struct *newmm)
 {
-	newmm->uprobes_state.xol_area = NULL;
-
 	if (test_bit(MMF_HAS_UPROBES, &oldmm->flags)) {
 		set_bit(MMF_HAS_UPROBES, &newmm->flags);
 		/* unconditionally, dup_mmap() skips VM_DONTCOPY vmas */
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -781,6 +781,13 @@ static void mm_init_owner(struct mm_stru
 #endif
 }
 
+static void mm_init_uprobes_state(struct mm_struct *mm)
+{
+#ifdef CONFIG_UPROBES
+	mm->uprobes_state.xol_area = NULL;
+#endif
+}
+
 static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
 	struct user_namespace *user_ns)
 {
@@ -808,6 +815,7 @@ static struct mm_struct *mm_init(struct
 #if defined(CONFIG_TRANSPARENT_HUGEPAGE) && !USE_SPLIT_PMD_PTLOCKS
 	mm->pmd_huge_pte = NULL;
 #endif
+	mm_init_uprobes_state(mm);
 
 	if (current->mm) {
 		mm->flags = current->mm->flags & MMF_INIT_MASK;

  parent reply	other threads:[~2017-09-05  7:20 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-05  7:11 [PATCH 4.12 00/27] 4.12.11-stable review Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 01/27] arm64: mm: abort uaccess retries upon fatal signal Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 03/27] irqchip: mips-gic: SYNC after enabling GIC region Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 04/27] Input: synaptics - fix device info appearing different on reconnect Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 05/27] Input: xpad - fix PowerA init quirk for some gamepad models Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 06/27] crypto: chacha20 - fix handling of chunked input Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 07/27] i2c: ismt: Dont duplicate the receive length for block reads Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 08/27] i2c: ismt: Return EMSGSIZE for block reads with bogus length Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 09/27] crypto: algif_skcipher - only call put_page on referenced and used pages Greg Kroah-Hartman
2017-09-05  7:11 ` Greg Kroah-Hartman [this message]
2017-09-05  7:11 ` [PATCH 4.12 11/27] mm, madvise: ensure poisoned pages are removed from per-cpu lists Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 12/27] ceph: fix readpage from fscache Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 13/27] cpumask: fix spurious cpumask_of_node() on non-NUMA multi-node configs Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 14/27] cpuset: Fix incorrect memory_pressure control file mapping Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 15/27] alpha: uapi: Add support for __SANE_USERSPACE_TYPES__ Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 16/27] CIFS: Fix maximum SMB2 header size Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 17/27] CIFS: remove endian related sparse warning Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 18/27] dm mpath: do not lock up a CPU with requeuing activity Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 19/27] drm/vmwgfx: Fix F26 Wayland screen update issue Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 20/27] wl1251: add a missing spin_lock_init() Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 21/27] mmc: sdhci-xenon: add set_power callback Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 22/27] lib/mpi: kunmap after finishing accessing buffer Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 23/27] xfrm: policy: check policy direction value Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 25/27] nvme: fix the definition of the doorbell buffer config support bit Greg Kroah-Hartman
2017-09-05  7:11 ` [PATCH 4.12 26/27] drm/nouveau/i2c/gf119-: add support for address-only transactions Greg Kroah-Hartman
2017-09-05 16:48 ` [PATCH 4.12 00/27] 4.12.11-stable review Guenter Roeck
2017-09-05 17:40   ` Greg Kroah-Hartman
2017-09-05 17:15 ` Shuah Khan
2017-09-05 17:40   ` Greg Kroah-Hartman

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=20170905070923.654450153@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=acme@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=dvyukov@google.com \
    --cc=ebiggers@google.com \
    --cc=koct9i@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mhocko@suse.com \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=vbabka@suse.cz \
    /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;
as well as URLs for NNTP newsgroup(s).