Bug 1230910 Remove some __try blocks and crash immediately
MozReview-Commit-ID: 1xFpduDtjCf
--- a/security/sandbox/chromium/sandbox/win/src/crosscall_params.h
+++ b/security/sandbox/chromium/sandbox/win/src/crosscall_params.h
@@ -235,22 +235,32 @@ class ActualCallParams : public CrossCal
// It does not fit, abort copy.
return false;
}
char* dest = reinterpret_cast<char*>(this) + param_info_[index].offset_;
// We might be touching user memory, this has to be done from inside a try
// except.
- __try {
+ /*
+ * This is a touch one, as this function is used in a lot (all?) IPC calls
+ * After spot-checking several of them, it seems that a failure here results
+ * in propogating an error up the stack and the operation simply fails and
+ * whatever expected it to succeed is just going to continue on and
+ * maybe/probably cause an error later on.
+ *
+ * So commenting this out makes me nervous, and we'll probably just have
+ * to do rigorous testing.
+ */
+ //__try {
memcpy(dest, parameter_address, size);
- }
- __except(EXCEPTION_EXECUTE_HANDLER) {
- return false;
- }
+ //}
+ //__except(EXCEPTION_EXECUTE_HANDLER) {
+ //return false;
+ //}
// Set the flag to tell the broker to update the buffer once the call is
// made.
if (is_in_out)
SetIsInOut(true);
param_info_[index + 1].offset_ = Align(param_info_[index].offset_ +
size);
--- a/security/sandbox/chromium/sandbox/win/src/crosscall_server.cc
+++ b/security/sandbox/chromium/sandbox/win/src/crosscall_server.cc
@@ -125,17 +125,26 @@ CrossCallParamsEx* CrossCallParamsEx::Cr
char* backing_mem = NULL;
uint32_t param_count = 0;
uint32_t declared_size;
uint32_t min_declared_size;
CrossCallParamsEx* copied_params = NULL;
// Touching the untrusted buffer is done under a SEH try block. This
// will catch memory access violations so we don't crash.
- __try {
+
+ /*
+ * An exception results in a 'false' being returned from InvokeCallback
+ * to indicate an error condition. This 'false' is ignored, and operations
+ * continue as if there was no error, and presumably cause an error at
+ * some other time.
+ *
+ * Remove the __try/__except and crash immediately. Just have to test...
+ */
+ //__try {
CrossCallParams* call_params =
reinterpret_cast<CrossCallParams*>(buffer_base);
// Check against the minimum size given the number of stated params
// if too small we bail out.
param_count = call_params->GetParamsCount();
min_declared_size = sizeof(CrossCallParams) +
((param_count + 1) * sizeof(ParamInfo));
@@ -163,22 +172,22 @@ CrossCallParamsEx* CrossCallParamsEx::Cr
// Check that the copied buffer is still valid.
if (copied_params->GetParamsCount() != param_count ||
GetActualBufferSize(param_count, backing_mem) != declared_size ||
!IsSizeWithinRange(buffer_size, min_declared_size, declared_size)) {
delete [] backing_mem;
return NULL;
}
- } __except(EXCEPTION_EXECUTE_HANDLER) {
+ //} __except(EXCEPTION_EXECUTE_HANDLER) {
// In case of a windows exception we know it occurred while touching the
// untrusted buffer so we bail out as is.
- delete [] backing_mem;
- return NULL;
- }
+ //delete [] backing_mem;
+ //return NULL;
+ //}
const char* last_byte = &backing_mem[declared_size];
const char* first_byte = &backing_mem[min_declared_size];
// Verify here that all and each parameters make sense. This is done in the
// local copy.
for (uint32_t ix = 0; ix != param_count; ++ix) {
uint32_t size = 0;