| 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
| ||
|
Message-ID: <20080304041230.GA22753@Krystal>
Date: Mon, 3 Mar 2008 23:12:30 -0500
From: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
To: Jeremy Fitzhardinge <jeremy@...p.org>
Cc: Andi Kleen <ak@...e.de>, "H. Peter Anvin" <hpa@...or.com>,
Ingo Molnar <mingo@...e.hu>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Zachary Amsden <zach@...are.com>
Subject: Re: bad paravirt/Xen interaction in "x86 - Enhance DEBUG_RODATA
support - alternatives"
* Jeremy Fitzhardinge (jeremy@...p.org) wrote:
> Mathieu Desnoyers wrote:
>> * Jeremy Fitzhardinge (jeremy@...p.org) wrote:
>>
>>> Andi Kleen wrote:
>>>
>>>> On Monday 03 March 2008 18:58:03 Jeremy Fitzhardinge wrote:
>>>>
>>>>
>>>>> Perhaps, though that's uncached by default.
>>>>>
>>>> ioremap_cached()
>>>>
>>> Sure. But given that from the perspective of this problem ioremap* is
>>> just a wrapper for vmap, we may as well use it directly and avoid getting
>>> tangled up in any current or future io-related stuff that ioremap may
>>> want to do.
>>>
>>> J
>>>
>>
>> Would you have a quick hint on why I get a page fault with the following
>> implementation ? There is probably a fundamental detail I missed.
>>
>>
>> void *__kprobes text_poke(void *addr, const void *opcode, size_t len)
>> {
>> char *vaddr;
>> struct page *pages[1];
>>
>> BUG_ON(len > sizeof(long));
>> BUG_ON((((long)addr + len - 1) & ~(sizeof(long) - 1))
>> - ((long)addr & ~(sizeof(long) - 1)));
>> if (kernel_text_address((unsigned long)addr))
>> pages[0] = virt_to_page(addr);
>> else
>> vaddr = addr;
>>
>
Hi Jeremy,
(Connecting brain...)
> What's this for? You just overwrite vaddr with the vmap on the next line.
>
Ah, it's supposed to be
if (kernel_text_address((unsigned long)addr)) {
pages[0] = virt_to_page(addr);
vaddr = vmap(pages, 1, VM_MAP, PAGE_KERNEL);
memcpy(&vaddr[(unsigned long)addr & ~PAGE_MASK], opcode, len);
} else
memcpy(addr, opcode, len);
>> vaddr = vmap(pages, 1, VM_MAP, PAGE_KERNEL);
>>
> Do you need to handle the case of an instruction spanning a page boundary?
>
Not really :
1-byte changes :
- kprobes is a 1-byte breakpoint
- smp alternatives changes an f0 prefix into a 1-byte nop when a single
CPU is active (the opposite is done when bringing up a second CPU).
Aligned on word size :
- immediate values are aligned on word size
At boot time only, before kernel pages are marked RO :
- alternatives and paravirt are applied at boot time only
But yes, I guess it's cheap and allows supporting users which does not
have their instructions aligned. I'll change it.
>> memcpy(&vaddr[(unsigned long)addr & PAGE_MASK], opcode, len);
>> if (kernel_text_address((unsigned long)addr))
>> 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;
>> }
>>
>>
>> [ 0.149856] SMP alternatives: switching to UP code
>> [ 0.152009] BUG: unable to handle kernel paging request at
>> b8902000
> That's a userspace address, unless you've got 2G:2G, and the error code
> says there's nothing mapped there.
>
<banging head on the wall> & PAGE_MASK -> & ~PAGE_MASK :)
Thanks!
Mathieu
>> [ 0.152009] IP: [<c03acfa1>] text_poke+0x85/0xb4
>> [ 0.152009] *pde = 00000000
>> [ 0.152009] Oops: 0002 [#1] SMP
>> [ 0.152009] LTT NESTING LEVEL : 0 <0>
>> [ 0.152009] Modules linked in:
>> [ 0.152009] [ 0.152009] Pid: 0, comm: swapper Not tainted
>> (2.6.25-rc3-testssmp #744)
>> [ 0.152009] EIP: 0060:[<c03acfa1>] EFLAGS: 00010002 CPU: 0
>> [ 0.152009] EIP is at text_poke+0x85/0xb4
>> [ 0.152009] EAX: f8800000 EBX: 00000001 ECX: 00000001 EDX: f8800000
>> [ 0.152009] ESI: c04a3fab EDI: b8902000 EBP: c0102114 ESP: c04a3f88
>> [ 0.152009] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
>> [ 0.152009] Process swapper (pid: 0, ti=c04a2000 task=c04703a0
>> task.ti=c04a2)
>> [ 0.152009] Stack: 00000163 f8800000 c1002040 c04a400c c04a7cac
>> c0100000 c03 [ 0.152009] 90411244 00000206 c04e072c c17fb72c
>> 0000672c c04aaefe c03 [ 0.152009] c04ab76a c17f5000 c04a896a
>> 00000092 c04a80d7 00000008 000 [ 0.152009] Call Trace:
>> [ 0.152009] [<c03b0139>]
>> _etext+0x0/0xf7ec7 [ 0.152009]
>> [<c0107814>] alternatives_smp_unlock+0x57/0x59 [
>> 0.152009] [<c04aaefe>] alternative_instructions+0x152/0x157
>> [ 0.152009] [<c03b0139>] _etext+0x0/0xf7ec7
>> [ 0.152009] [<c04ab76a>] check_bugs+0x131/0x14e
>> [ 0.152009] [<c04a896a>] start_kernel+0x2d1/0x327
>> [ 0.152009] [<c04a80d7>] unknown_bootoption+0x0/0x1e3
>> [ 0.152009] =======================
>> [ 0.152009] Code: b9 04 00 00 00 ba 01 00 00
>> 00 e8 a6 87 db ff 89 44 24 04 8 [ 0.152009] EIP: [<c03acfa1>]
>> text_poke+0x85/0xb4 SS:ESP 0068:c04a3f88 [ 0.152009] ---[ end
>> trace ca143223eefdc828 ]--- [ 0.152009]
>> Kernel panic - not syncing: Attempted to kill the idle task!
>
> J
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
--
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/
Powered by blists - more mailing lists