linux-kernel - Re: [PATCH v4 3.0-rc2-tip 2/22] 2: uprobes: Breakground page replacement.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 14 Jun 2011 18:05:30 +0530
From:	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...e.hu>,
	Steven Rostedt <rostedt@...dmis.org>,
	Linux-mm <linux-mm@...ck.org>,
	Arnaldo Carvalho de Melo <acme@...radead.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Hugh Dickins <hughd@...gle.com>,
	Christoph Hellwig <hch@...radead.org>,
	Andi Kleen <andi@...stfloor.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Jonathan Corbet <corbet@....net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jim Keniston <jkenisto@...ux.vnet.ibm.com>,
	Roland McGrath <roland@...k.frob.com>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 3.0-rc2-tip 2/22]  2: uprobes: Breakground page
 replacement.

> > +static int write_opcode(struct task_struct *tsk, struct uprobe * uprobe,
> > +			unsigned long vaddr, uprobe_opcode_t opcode)
> > +{
> > +	struct page *old_page, *new_page;
> > +	void *vaddr_old, *vaddr_new;
> > +	struct vm_area_struct *vma;
> > +	unsigned long addr;
> > +	int ret;
> > +
> > +	/* Read the page with vaddr into memory */
> > +	ret = get_user_pages(tsk, tsk->mm, vaddr, 1, 1, 1, &old_page, &vma);
> 
> Sorry if this was already discussed... But why we are using FOLL_WRITE here?
> We are not going to write into this page, and this provokes the unnecessary
> cow, no?

Yes, We are not going to write to the page returned by get_user_pages
but a copy of that page. The idea was if we cow the page then we dont
need to cow it at the replace_page time and since get_user_pages knows
the right way to cow the page, we dont have to write another routine to
cow the page.

I am still not clear on your concern.

Is it that we should delay cowing the page to the time we actually write
into the page? 

or

Is it that we dont need to cow at all if we are replacing a file backed
page with anon page?

I think we have to cow the page either at page replacement time or at
the beginning. I had tried the option of not cowing the page and it
failed but I dont recollect why it failed but back then we used
write_protect_page and replace_page from ksm.c

> 
> Also. This is called under down_read(mmap_sem), can't we race with
> access_process_vm() modifying the same memory?

Yes, we could be racing with access_process_vm on the same memory.

Do we have any other option other than making write_opcode/read_opcode
being called under down_write(mmap_sem)? I know that write_opcode worked
when we take down_write(mmap_sem). Just that 
anon_vma_prepare() documents that it should be called under read lock
for mmap_sem.

Also Thomas had once asked why we were calling it under down_write.
May be race with access_process_vm is a good enough reason to call it
with down_write.

-- 
Thanks and Regards
Srikar

-- 
Thanks and Regards
Srikar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/