All of lore.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, 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.