Skip to content

Commit

Permalink
Mojo: Add checks to test for UAF
Browse files Browse the repository at this point in the history
Adds a temporary sentinel value to WatcherDispatcher which is mutated
only upon destruction. Adds some corresponding CHECKs to investigate
whether crashes a result of UAF.

BUG=740044
TBR=jcivelli@chromium.org

Change-Id: I9ef65c3b149a60d5c95341e07b4d9b371fc84695
Reviewed-on: https://chromium-review.googlesource.com/630438
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498263}
  • Loading branch information
krockot authored and Commit Bot committed Aug 29, 2017
1 parent d2c26a3 commit ac4fbdf
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
21 changes: 20 additions & 1 deletion mojo/edk/system/watcher_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <limits>
#include <map>

#include "base/debug/alias.h"
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "mojo/edk/system/watch.h"
Expand All @@ -25,6 +26,11 @@ void WatcherDispatcher::NotifyHandleState(Dispatcher* dispatcher,
if (it == watched_handles_.end())
return;

// TODO(crbug.com/740044): Remove this.
uint32_t sentinel = sentinel_value_for_debugging_;
base::debug::Alias(&sentinel);
CHECK_EQ(0x12345678u, sentinel);

// Maybe fire a notification to the watch associated with this dispatcher,
// provided we're armed and it cares about the new state.
if (it->second->NotifyState(state, armed_)) {
Expand Down Expand Up @@ -53,6 +59,11 @@ void WatcherDispatcher::NotifyHandleClosed(Dispatcher* dispatcher) {
watched_handles_.erase(it);
}

// TODO(crbug.com/740044): Remove this.
uint32_t sentinel = sentinel_value_for_debugging_;
base::debug::Alias(&sentinel);
CHECK_EQ(0x12345678u, sentinel);

// NOTE: It's important that this is called outside of |lock_| since it
// acquires internal Watch locks.
watch->Cancel();
Expand Down Expand Up @@ -174,6 +185,11 @@ MojoResult WatcherDispatcher::CancelWatch(uintptr_t context) {
watches_.erase(it);
}

// TODO(crbug.com/740044): Remove this.
uint32_t sentinel = sentinel_value_for_debugging_;
base::debug::Alias(&sentinel);
CHECK_EQ(0x12345678u, sentinel);

// Mark the watch as cancelled so no further notifications get through.
watch->Cancel();

Expand Down Expand Up @@ -253,7 +269,10 @@ MojoResult WatcherDispatcher::Arm(
return MOJO_RESULT_FAILED_PRECONDITION;
}

WatcherDispatcher::~WatcherDispatcher() = default;
WatcherDispatcher::~WatcherDispatcher() {
// TODO(crbug.com/740044): Remove this.
sentinel_value_for_debugging_ = 0x87654321;
}

} // namespace edk
} // namespace mojo
5 changes: 5 additions & 0 deletions mojo/edk/system/watcher_dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef MOJO_EDK_SYSTEM_WATCHER_DISPATCHER_H_
#define MOJO_EDK_SYSTEM_WATCHER_DISPATCHER_H_

#include <stdint.h>

#include <map>
#include <set>

Expand Down Expand Up @@ -93,6 +95,9 @@ class WatcherDispatcher : public Dispatcher {
// an invalid object. It must therefore never be dereferenced.
const Watch* last_watch_to_block_arming_ = nullptr;

// TODO(crbug.com/740044): Remove this.
uint32_t sentinel_value_for_debugging_ = 0x12345678;

DISALLOW_COPY_AND_ASSIGN(WatcherDispatcher);
};

Expand Down

0 comments on commit ac4fbdf

Please sign in to comment.