All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	"H. Peter Anvin" <hpa@zytor.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	Andi Kleen <andi@firstfloor.org>, Borislav Petkov <bp@alien8.de>,
	Hugh Dickins <hughd@google.com>, Ingo Molnar <mingo@kernel.org>,
	Jiri Kosina <jkosina@suse.cz>,
	Peter Zijlstra <peterz@infradead.org>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 0/1] uprobes: Kill __replace_page(), change uprobe_write_opcode() to rely on gup(WRITE)
Date: Mon, 9 Dec 2013 22:18:24 +0100	[thread overview]
Message-ID: <20131209211824.GA15006@redhat.com> (raw)

Hello.

It is not clear to me if Linus still dislikes this change or not.
Let me send the patch "officially" so that it can be nacked if I
misunderstood the result of discussion.

Changes:

	- add a huge comment above gup(WRITE | FORCE)

	- add WARN_ON(!(PageAnon() && page_mapcount() == 1))
	  to ensure it works as expected

If (say, on x86) we can avoid the pte games, we can simply add

	if (IS_ENABLED(CONFIG_WHATEVER)) {
		copy_to_page(...);
		set_page_dirty_locked(page);
		goto put;
	}

right after the 2nd get_user_pages().

In any case I believe it would be very nice to kill __replace_page(),
and even the fact this patch removes include(mm/internal.h) makes me
think this patch makes sense. Assuming it is correct.

Oleg.
---

int uprobe_write_opcode(struct mm_struct *mm, unsigned long vaddr,
			uprobe_opcode_t opcode)
{
	struct page *page;
	struct vm_area_struct *vma;
	pte_t *ptep, entry;
	spinlock_t *ptlp;
	int ret;

	/* Read the page with vaddr into memory */
	ret = get_user_pages(NULL, mm, vaddr, 1, 0, 1, &page, NULL);
	if (ret < 0)
		return ret;

	ret = verify_opcode(page, vaddr, &opcode);
	if (ret <= 0)
		goto put;

 retry:
	put_page(page);
	/*
	 * Break the mapping unless the page is already anonymous and
	 * unshare the page, see the WARN_ON() below.
	 *
	 * We never write to the VM_SHARED vma, every caller must check
	 * valid_vma(). FOLL_WRITE | FOLL_FORCE should anonymize this
	 * page unless uprobe_write_opcode() was already called in the
	 * past or the application itself did mprotect(PROT_WRITE) and
	 * wrote into this page.
	 *
	 * If it was already anonymous it can be shared due to dup_mm(),
	 * in this case do_wp_page() or do_swap_page() will do another
	 * cow to unshare, so we can safely modify it.
	 */
	ret = get_user_pages(NULL, mm, vaddr, 1, 1, 1, &page, &vma);
	if (ret < 0)
		return ret;

	ptep = page_check_address(page, mm, vaddr, &ptlp, 0);
	if (!ptep)
		goto retry;

	ret = 0;
	if (WARN_ON(!PageAnon(page) || page_mapcount(page) != 1)) {
		dump_page(page);
		ret = -EFAULT;
		goto unlock;
	}

	/* Unmap this page to ensure that nobody can execute it */
	flush_cache_page(vma, vaddr, pte_pfn(*ptep));
	entry = ptep_clear_flush(vma, vaddr, ptep);

	/* Nobody can fault in this page, modify it */
	copy_to_page(page, vaddr, &opcode, UPROBE_SWBP_INSN_SIZE);

	/* Restore the old mapping */
	entry = pte_mkdirty(entry);
	flush_icache_page(vma, page);
	set_pte_at(mm, vaddr, ptep, entry);
	update_mmu_cache(vma, vaddr, ptep);
 unlock:
	pte_unmap_unlock(ptep, ptlp);
 put:
	put_page(page);
	return ret;
}


             reply	other threads:[~2013-12-09 21:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-09 21:18 Oleg Nesterov [this message]
2013-12-09 21:18 ` [PATCH 1/1] uprobes: Kill __replace_page(), change uprobe_write_opcode() to rely on gup(WRITE) Oleg Nesterov
2013-12-10  2:08 ` [PATCH 0/1] " Linus Torvalds
2013-12-10 19:18   ` Oleg Nesterov
2013-12-10 19:38     ` Linus Torvalds
2013-12-10 20:04       ` Oleg Nesterov
2013-12-10 20:16         ` Linus Torvalds
2013-12-10 21:24           ` Oleg Nesterov
2013-12-10 20:16         ` Oleg Nesterov
2013-12-10 20:19           ` Linus Torvalds

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=20131209211824.GA15006@redhat.com \
    --to=oleg@redhat.com \
    --cc=ananth@in.ibm.com \
    --cc=andi@firstfloor.org \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=hughd@google.com \
    --cc=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=torvalds@linux-foundation.org \
    /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.