This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH] x86 - Enhance DEBUG_RODATA support - alternatives
- From: Mathieu Desnoyers <mathieu dot desnoyers at polymtl dot ca>
- To: pageexec at freemail dot hu
- Cc: Ingo Molnar <mingo at elte dot hu>, Srinivasa DS <srinivasa at in dot ibm dot com>, Andrew Morton <akpm at linux-foundation dot org>, linux-kernel at vger dot kernel dot org, ananth at in dot ibm dot com, Jim Keniston <jkenisto at us dot ibm dot com>, srikar at linux dot vnet dot ibm dot com, SystemTAP <systemtap at sources dot redhat dot com>, Andi Kleen <andi at firstfloor dot org>, Thomas Gleixner <tglx at linutronix dot de>, Ingo Molnar <mingo at redhat dot com>, "H. Peter Anvin" <hpa at zytor dot com>, Jeremy Fitzhardinge <jeremy at goop dot org>, Arjan van de Ven <arjan at infradead dot org>
- Date: Thu, 6 Mar 2008 12:25:09 -0500
- Subject: Re: [PATCH] x86 - Enhance DEBUG_RODATA support - alternatives
- References: <20080306140105.GC28438@elte.hu> <47D014E1.30964.22A9390F@pageexec.freemail.hu>
* pageexec@freemail.hu (pageexec@freemail.hu) wrote:
> On 6 Mar 2008 at 15:01, Ingo Molnar wrote:
>
> > +/**
> > + * text_poke - Update instructions on a live kernel
> > + * @addr: address to modify
> > + * @opcode: source of the copy
> > + * @len: length to copy
> > + *
> > + * Only atomic text poke/set should be allowed when not doing early patching.
> > + * It means the size must be writable atomically and the address must be aligned
> > + * in a way that permits an atomic write. It also makes sure we fit on a single
> > + * page.
> > + */
> > +void *__kprobes text_poke(void *addr, const void *opcode, size_t len)
> > +{
> > + unsigned long flags;
> > + char *vaddr;
> > + int nr_pages = 2;
> > +
> > + BUG_ON(len > sizeof(long));
> > + BUG_ON((((long)addr + len - 1) & ~(sizeof(long) - 1))
> > + - ((long)addr & ~(sizeof(long) - 1)));
> > + {
> > + struct page *pages[2] = { virt_to_page(addr),
> > + virt_to_page(addr + PAGE_SIZE) };
I just want to ask for confirmation about two things :
First, that calling this text_poke implementation to modify text in a
module won't fail. Is virt_to_page(addr) ok if addr is in a vmalloc'ed
area ?
Second, that calling virt_to_page(addr + PAGE_SIZE) won't have
undesirable side-effects if addr happens to be in the last page of an
allocated range. It should be ok for the core kernel text, because it is
followed by the kernel rodata, but I am not certain for modules.
If any of these two are not true, then we should add a test for
if (kernel_text_address(addr)) {
around the vmap() allocation and fallback on
} else
vaddr = addr;
in the cases where text_poke is called to modify module code.
Mathieu
> > + if (!pages[1])
> > + nr_pages = 1;
> > + vaddr = vmap(pages, nr_pages, VM_MAP, PAGE_KERNEL);
> > + WARN_ON(!vaddr);
> > + local_irq_save(flags);
> > + memcpy(&vaddr[(unsigned long)addr & ~PAGE_MASK], opcode, len);
> > + local_irq_restore(flags);
>
> you probably want a BUG_ON instead (or some graceful recovery) else
> the NULL deref will trigger with IRQs off...
>
> > + vunmap(vaddr);
> > + }
> > sync_core();
> > /* Could also do a CLFLUSH here to speed up CPU recovery; but
> > that causes hangs on some VIA CPUs. */
> > + return addr;
> > }
>
>
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68