[plcrashreporter] 2 new revisions pushed by landon.j.fuller@gmail.com on 2013-04-22 20:50 GMT

[codesite...@google.com]

2 new revisions:

Revision: e7243654d3f2
Author: Landon Fuller <lan...@plausible.coop>
Date: Mon Apr 22 12:50:09 2013
Log: Disable test after build; this makes it easier to debug issues in
the ...
http://code.google.com/p/plcrashreporter/source/detail?r=e7243654d3f2

Revision: a24b93f01a87
Author: Landon Fuller <lan...@plausible.coop>
Date: Mon Apr 22 13:49:27 2013
Log: Add support for returning mobject mappings shorter than the
requested ...
http://code.google.com/p/plcrashreporter/source/detail?r=a24b93f01a87

==============================================================================
Revision: e7243654d3f2
Author: Landon Fuller <lan...@plausible.coop>
Date: Mon Apr 22 12:50:09 2013
Log: Disable test after build; this makes it easier to debug issues in
the unit tests.

http://code.google.com/p/plcrashreporter/source/detail?r=e7243654d3f2

Modified:
/CrashReporter.xcodeproj/project.pbxproj

=======================================
--- /CrashReporter.xcodeproj/project.pbxproj Thu Feb 14 10:00:07 2013
+++ /CrashReporter.xcodeproj/project.pbxproj Mon Apr 22 12:50:09 2013
@@ -2985,7 +2985,6 @@
PRODUCT_NAME = "Tests-MacOSX";
SDKROOT = macosx;
SKIP_INSTALL = YES;
- TEST_AFTER_BUILD = YES;
WARNING_CFLAGS = (
"$(inherited)",
"$(OTHER_WARNING_NO_ADDRESS)",
@@ -3018,7 +3017,6 @@
PRODUCT_NAME = "Tests-MacOSX";
SDKROOT = macosx;
SKIP_INSTALL = YES;
- TEST_AFTER_BUILD = YES;
WARNING_CFLAGS = (
"$(inherited)",
"$(OTHER_WARNING_NO_ADDRESS)",

==============================================================================
Revision: a24b93f01a87
Author: Landon Fuller <lan...@plausible.coop>
Date: Mon Apr 22 13:49:27 2013
Log: Add support for returning mobject mappings shorter than the
requested length.

Due to bugs in the update_dyld_shared_cache(1), the segment vmsize
defined in the Mach-O load commands may be invalid, and the
declared size may be unmappable.

This bug appears to be caused by a bug in computing the correct vmsize
when update_dyld_shared_cache(1) generates the single shared LINKEDIT
segment, and has been reported to Apple as rdar://13707406.

http://code.google.com/p/plcrashreporter/source/detail?r=a24b93f01a87

Modified:
/Source/PLCrashAsyncMObject.c
/Source/PLCrashAsyncMObject.h
/Source/PLCrashAsyncMObjectTests.m
/Source/PLCrashAsyncMachOImage.c
/Source/PLCrashAsyncMachOString.c

=======================================
--- /Source/PLCrashAsyncMObject.c Thu Apr 18 09:02:16 2013
+++ /Source/PLCrashAsyncMObject.c Mon Apr 22 13:49:27 2013
@@ -49,24 +49,27 @@
* @param task The task from which the memory will be mapped.
* @param task_address The task-relative address of the memory to be
mapped. This is not required to fall on a page boundry.
* @param length The total size of the mapping to create.
+ * @param require_full If false, short mappings will be permitted in the
case where a memory object of the requested length
+ * does not exist at the target address. It is the caller's responsibility
to validate the resulting length of the
+ * mapping, eg, using plcrash_async_mobject_remap_address() and similar.
If true, and the entire requested page range is
+ * not valid, the mapping request will fail.
*
* @return On success, returns PLCRASH_ESUCCESS. On failure, one of the
plcrash_error_t error values will be returned, and no
* mapping will be performed.
*
* @warn Callers must call plcrash_async_mobject_free() on @a mobj, even
if plcrash_async_mobject_init() fails.
*/
-plcrash_error_t plcrash_async_mobject_init (plcrash_async_mobject_t *mobj,
mach_port_t task, pl_vm_address_t task_addr, pl_vm_size_t length) {
+plcrash_error_t plcrash_async_mobject_init (plcrash_async_mobject_t *mobj,
mach_port_t task, pl_vm_address_t task_addr, pl_vm_size_t length, bool
require_full) {
/* We must first initialize vm_address to 0x0. We'll check this in
_free() to determine whether calling vm_deallocate() is required */
mobj->vm_address = 0x0;

kern_return_t kt;

/* Compute the total required page size. */
- pl_vm_size_t page_size = mach_vm_round_page(length + (task_addr -
mach_vm_trunc_page(task_addr)));
-
- /* Remap the target pages into our process */
+ pl_vm_address_t page_addr = mach_vm_trunc_page(task_addr);
+ pl_vm_size_t page_size = mach_vm_round_page(length + (task_addr -
page_addr));

-
+ /* Remap the target pages into our process */
#ifdef PL_HAVE_BROKEN_VM_REMAP
/* Memory object implementation */
memory_object_size_t entry_length = page_size;
@@ -76,6 +79,21 @@
PLCF_DEBUG("mach_make_memory_entry_64() failed: %d", kt);
return PLCRASH_ENOMEM;
}
+
+ /*
+ * If short mappings are enabled, and the entry is smaller than the
target mapping, use the memory entry's
+ * size rather than the originally requested size. Otherwise, a
smaller entry will result in a vm_map()
+ * error when the requested pages are unavailable.
+ */
+ if (!require_full && entry_length < page_size) {
+ /* Reset the page size to match the actual available memory ... */
+ page_size = entry_length;
+
+ /* The length represents the user's requested byte length, and is
not required to be page aligned. Thus, it
+ * needs to be recomputed such that it fits within the smaller
entry size here, while still accounting for the
+ * offset of the user's non-page-aligned start address. */
+ length = entry_length - (task_addr - page_addr);
+ }

#ifdef PL_HAVE_MACH_VM
kt = mach_vm_map(mach_task_self(), &mobj->vm_address, page_size, 0x0,
VM_FLAGS_ANYWHERE, mem_handle, 0x0, TRUE, VM_PROT_READ, VM_PROT_READ,
VM_INHERIT_COPY);
=======================================
--- /Source/PLCrashAsyncMObject.h Tue Jan 1 14:02:34 2013
+++ /Source/PLCrashAsyncMObject.h Mon Apr 22 13:49:27 2013
@@ -59,7 +59,7 @@
pl_vm_address_t vm_address;
} plcrash_async_mobject_t;

-plcrash_error_t plcrash_async_mobject_init (plcrash_async_mobject_t *mobj,
mach_port_t task, pl_vm_address_t task_addr, pl_vm_size_t length);
+plcrash_error_t plcrash_async_mobject_init (plcrash_async_mobject_t *mobj,
mach_port_t task, pl_vm_address_t task_addr, pl_vm_size_t length, bool
require_full);

bool plcrash_async_mobject_verify_local_pointer (plcrash_async_mobject_t
*mobj, uintptr_t address, size_t length);
void *plcrash_async_mobject_remap_address (plcrash_async_mobject_t *mobj,
pl_vm_address_t address, size_t length);
=======================================
--- /Source/PLCrashAsyncMObjectTests.m Tue Jan 1 14:02:34 2013
+++ /Source/PLCrashAsyncMObjectTests.m Mon Apr 22 13:49:27 2013
@@ -47,7 +47,7 @@

/* Map the memory */
plcrash_async_mobject_t mobj;
- STAssertEquals(PLCRASH_ESUCCESS, plcrash_async_mobject_init(&mobj,
mach_task_self(), (pl_vm_address_t)template, size), @"Failed to initialize
mapping");
+ STAssertEquals(PLCRASH_ESUCCESS, plcrash_async_mobject_init(&mobj,
mach_task_self(), (pl_vm_address_t)template, size, true), @"Failed to
initialize mapping");

/* Verify the mapped data */
STAssertTrue(memcmp((void *)mobj.address, template, size) == 0,
@"Mapping appears to be incorrect");
@@ -72,7 +72,7 @@

/* Map the memory */
plcrash_async_mobject_t mobj;
- STAssertEquals(PLCRASH_ESUCCESS, plcrash_async_mobject_init(&mobj,
mach_task_self(), (pl_vm_address_t)template, size), @"Failed to initialize
mapping");
+ STAssertEquals(PLCRASH_ESUCCESS, plcrash_async_mobject_init(&mobj,
mach_task_self(), (pl_vm_address_t)template, size, true), @"Failed to
initialize mapping");
STAssertEquals((pl_vm_address_t)template, (pl_vm_address_t)
(mobj.address + mobj.vm_slide), @"Incorrect slide value!");

/* Test slide handling */
@@ -91,7 +91,7 @@

/* Map the memory */
plcrash_async_mobject_t mobj;
- STAssertEquals(PLCRASH_ESUCCESS, plcrash_async_mobject_init(&mobj,
mach_task_self(), (pl_vm_address_t)template, size), @"Failed to initialize
mapping");
+ STAssertEquals(PLCRASH_ESUCCESS, plcrash_async_mobject_init(&mobj,
mach_task_self(), (pl_vm_address_t)template, size, true), @"Failed to
initialize mapping");

/* Test the address range validation */
STAssertFalse(plcrash_async_mobject_verify_local_pointer(&mobj,
mobj.address-1, 10), @"Returned pointer for a range that starts before our
memory object");
=======================================
--- /Source/PLCrashAsyncMachOImage.c Wed Mar 27 12:39:50 2013
+++ /Source/PLCrashAsyncMachOImage.c Mon Apr 22 13:49:27 2013
@@ -156,7 +156,7 @@
pl_vm_size_t cmd_len = image->swap32(image->header.sizeofcmds);
pl_vm_size_t cmd_offset = image->header_addr + image->header_size;
image->ncmds = image->swap32(image->header.ncmds);
- plcrash_error_t ret = plcrash_async_mobject_init(&image->load_cmds,
image->task, cmd_offset, cmd_len);
+ plcrash_error_t ret = plcrash_async_mobject_init(&image->load_cmds,
image->task, cmd_offset, cmd_len, true);
if (ret != PLCRASH_ESUCCESS) {
PLCF_DEBUG("Failed to map Mach-O load commands in image %s",
image->name);
return ret;
@@ -361,6 +361,13 @@
* @param seg The segment data to be initialized. It is the caller's
responsibility to dealloc @a seg after
* a successful initialization.
*
+ * @warning Due to bugs in the update_dyld_shared_cache(1), the segment
vmsize defined in the Mach-O load commands may
+ * be invalid, and the declared size may be unmappable. As such, it is
possible that this function will return a mapping
+ * that is less than the total requested size. All accesses to this
mapping should be done (as is already the norm)
+ * through range-checked pointer validation. This bug appears to be caused
by a bug in computing the correct vmsize
+ * when update_dyld_shared_cache(1) generates the single shared LINKEDIT
segment, and has been reported to Apple
+ * as rdar://13707406.
+ *
* @return Returns PLCRASH_ESUCCESS on success, or an error result on
failure.
*/
plcrash_error_t plcrash_async_macho_map_segment (plcrash_async_macho_t
*image, const char *segname, pl_async_macho_mapped_segment_t *seg) {
@@ -391,8 +398,8 @@
seg->filesize = image->swap32(cmd_32->filesize);
}

- /* Perform and return the mapping */
- return plcrash_async_mobject_init(&seg->mobj, image->task, segaddr,
segsize);
+ /* Perform and return the mapping (permitting shorter mappings, as
documented above). */
+ return plcrash_async_mobject_init(&seg->mobj, image->task, segaddr,
segsize, false);
}

/**
@@ -467,7 +474,7 @@


/* Perform and return the mapping */
- return plcrash_async_mobject_init(mobj, image->task, sectaddr,
sectsize);
+ return plcrash_async_mobject_init(mobj, image->task, sectaddr,
sectsize, true);
}
}

@@ -582,7 +589,7 @@
return PLCRASH_ENOTFOUND;
}

- /* Map in the __LINKEDIT segment, which includes the symbol and string
tables */
+ /* Map in the __LINKEDIT segment, which includes the symbol and string
tables. */
pl_async_macho_mapped_segment_t linkedit_seg;
plcrash_error_t err =
plcrash_async_macho_map_segment(image, "__LINKEDIT", &linkedit_seg);
if (err != PLCRASH_ESUCCESS) {
=======================================
--- /Source/PLCrashAsyncMachOString.c Thu Apr 18 09:18:32 2013
+++ /Source/PLCrashAsyncMachOString.c Mon Apr 22 13:49:27 2013
@@ -56,9 +56,10 @@

pl_vm_address_t cursor = string->address;

- /* Map in the page containing the string, +1 up to one additional
page. */
+ /* Map in the page containing the string, +1 up to one additional
page. Short reads are permitted, as the next page
+ * may not be readable. */
size_t page_count = 1;
- plcrash_error_t err = plcrash_async_mobject_init(&string->mobj,
string->image->task, string->address, PAGE_SIZE);
+ plcrash_error_t err = plcrash_async_mobject_init(&string->mobj,
string->image->task, string->address, PAGE_SIZE, false);
if (err != PLCRASH_ESUCCESS)
return err;

@@ -70,9 +71,15 @@
PLCF_DEBUG("Mapped a string larger than one page!
Remapping ...");
page_count++;
plcrash_async_mobject_free(&string->mobj);
- err = plcrash_async_mobject_init(&string->mobj,
string->image->task, string->address, page_count*PAGE_SIZE);
+ err = plcrash_async_mobject_init(&string->mobj,
string->image->task, string->address, page_count*PAGE_SIZE, false);
if (err != PLCRASH_ESUCCESS)
return err;
+
+ p = plcrash_async_mobject_remap_address(&string->mobj, cursor,
1);
+ if (p == NULL) {
+ PLCF_DEBUG("Failed to remap additional space ...");
+ return PLCRASH_EINVAL;
+ }
}

c = *p;