[haiku-bugs] Re: [Haiku] #6271: support for images with a single rwx PT_LOAD program header
- From: "bonefish" <trac@xxxxxxxxxxxx>
- Date: Tue, 06 Jul 2010 20:12:01 -0000
#6271: support for images with a single rwx PT_LOAD program header
----------------------------+-----------------------------------------------
Reporter: lucian | Owner: axeld
Type: bug | Status: new
Priority: normal | Milestone: R1
Component: System/Kernel | Version: R1/Development
Resolution: | Keywords:
Blocked By: | Has a Patch: 1
Platform: All | Blocking:
----------------------------+-----------------------------------------------
Comment (by bonefish):
Replying to [comment:8 lucian]:
> I tested the latest patch on x86 with modules that have a single RWE
PT_LOAD header and two normal PT_LOAD headers.
>
> I assume this will work on PPC too, but it wouldn't hurt to try :)
With this patch when booting Haiku in qemu I get:
{{{
bfs: mounted "Haiku" (root node at 131072, device =
/dev/disk/ata/0/master/raw)
Mounted boot partition: /dev/disk/ata/0/master/raw
module: Search for file_cache/launch_speedup/v1 failed.
amiga_rdb: weird program header flags 0x6
vm_soft_fault: va 0x814ca000 not covered by area in address space
vm_page_fault: vm_soft_fault returned error 'Bad address' on fault at
0x814ca090, ip 0x8004bfcf, write 0, user 0, thread 0xb
PANIC: vm_page_fault: unhandled page fault in kernel space at 0x814ca090,
ip 0x8004bfcf
Welcome to Kernel Debugging Land...
Thread 11 "main2" running on CPU 0
stack trace for thread 11 "main2"
kernel stack: 0x8106b000 to 0x8106f000
frame caller <image>:function + offset
0 8106e798 (+ 32) 8010b70e <kernel_x86> arch_debug_stack_trace() +
0x0012
1 8106e7b8 (+ 16) 8007a1cf <kernel_x86> stack_trace_trampoline__FPv()
+ 0x000b
2 8106e7c8 (+ 12) 80110faa <kernel_x86>
arch_debug_call_with_fault_handler() + 0x001b
3 8106e7d4 (+ 48) 8007bc83 <kernel_x86>
debug_call_with_fault_handler() + 0x005b
4 8106e804 (+ 64) 8007a3f3 <kernel_x86>
kernel_debugger_loop__FPCcT0Pcl() + 0x021f
5 8106e844 (+ 48) 8007a754 <kernel_x86>
kernel_debugger_internal__FPCcT0Pcl() + 0x0048
6 8106e874 (+ 48) 8007bff4 <kernel_x86> panic() + 0x0024
7 8106e8a4 (+ 64) 800ef6a1 <kernel_x86> vm_page_fault() + 0x0135
8 8106e8e4 (+ 80) 8010d07e <kernel_x86>
page_fault_exception__FP6iframe() + 0x017a
9 8106e934 (+ 12) 80111f5d <kernel_x86> int_bottom() + 0x003d
kernel iframe at 0x8106e940 (end = 0x8106e990)
eax 0x814c8000 ebx 0x0 ecx 0x8207f140 edx 0x814ca090
esi 0xffffffff edi 0x80efe600 ebp 0x8106e998 esp 0x8106e974
eip 0x8004bfcf eflags 0x210246
vector: 0xe, error code: 0x0
10 8106e940 (+ 88) 8004bfcf <kernel_x86>
elf_parse_dynamic_section__FP14elf_image_info() + 0x002f
11 8106e998 (+ 128) 8004dc59 <kernel_x86> load_kernel_add_on() + 0x0491
12 8106ea18 (+ 48) 80053f49 <kernel_x86>
load_module_image__FPCcPP12module_image() + 0x006d
13 8106ea48 (+ 48) 800541dd <kernel_x86>
get_module_image__FPCcPP12module_image() + 0x0059
14 8106ea78 (+ 160) 80054bb0 <kernel_x86>
iterator_get_next_module__FP15module_iteratorPcPUl() + 0x0424
15 8106eb18 (+ 64) 8005721a <kernel_x86> read_next_module_name() +
0x0046
16 8106eb58 (+ 80) 800aa858 <kernel_x86>
_RescanDiskSystems__Q38BPrivate10DiskDevice18KDiskDeviceManagerRQ48BPrivate10DiskDevice18KDiskDeviceManager13DiskSystemMapb()
+ 0x00a0
17 8106eba8 (+ 112) 800aa9b2 <kernel_x86>
RescanDiskSystems__Q38BPrivate10DiskDevice18KDiskDeviceManager() + 0x0036
18 8106ec18 (+ 864) 800d0846 <kernel_x86> vfs_mount_boot_file_system() +
0x02be
19 8106ef78 (+ 96) 80053c04 <kernel_x86> main2__FPv() + 0x00a8
20 8106efd8 (+ 32) 8006a4b3 <kernel_x86>
_create_kernel_thread_kentry__Fv() + 0x001b
21 8106eff8 (+2130251784) 8006a450 <kernel_x86>
thread_kthread_exit__Fv() + 0x0000
}}}
Haven't tried to investigate what the issue is.
That aside, some coding style remarks:
{{{
@@ -2066,6 +2068,7 @@
B_PAGE_SIZE);
if (end > reservedSize)
reservedSize = end;
+ countExecutableHeaders +=
programHeaders[i].IsExecutable();
}}}
Adding a bool to an int is really not beautiful.
{{{
@@ -2105,7 +2108,14 @@
}
// we're here, so it must be a PT_LOAD segment
- if (programHeaders[i].IsReadWrite()) {
+
+ // Usually add-ons have two PT_LOAD headers: one for .data
one or .text.
+ // x86 and PPC may differ in permission bits for .data's
PT_LOAD header
+ // x86 is usually RW, PPC is RWE
}}}
Weird indentation in the comment.
{{{
+
+ // Some add-ons may have .text and .data concatenated in a
single
+ // PT_LOAD RWE header and we must map that to .text.
+ if (programHeaders[i].IsReadWrite() &&
(countExecutableHeaders > 1)) {
}}}
Superfluous parenthesis.
{{{
@@ -2122,6 +2132,10 @@
}
region = &image->text_region;
+ // some programs may have .text and .data
concatenated in a
+ // single PT_LOAD section which is
readable/writtable/executable
+ textSectionWritable =
programHeaders[i].IsReadWrite();
}}}
Typo: "writtable"
{{{
@@ -2191,9 +2205,12 @@
goto error5;
// We needed to read in the contents of the "text" area, but
- // now we can protect it read-only/execute
+ // now we can protect it read-only/execute, unless this is a
+ // special image with concatenated .text and .data, when it
+ // will also nead write access.
}}}
Typo: "nead"
{{{
Index: headers/private/system/elf32.h
===================================================================
--- headers/private/system/elf32.h (revision 37412)
+++ headers/private/system/elf32.h (working copy)
@@ -367,7 +367,7 @@
inline bool
Elf32_Phdr::IsExecutable() const
{
- return (p_flags & PF_PROTECTION_MASK) == (PF_READ | PF_EXECUTE);
+ return !!(p_flags & PF_EXECUTE);
}
}}}
Please use `(p_flags & PF_EXECUTE) != 0` respective `... == 0` to test
bits. Older Haiku code omits the `!= 0` respectively uses the `!`
operator, but new/touched code shouldn't.
--
Ticket URL: <http://dev.haiku-os.org/ticket/6271#comment:9>
Haiku <http://dev.haiku-os.org>
Haiku - the operating system.
Other related posts:
