Refactor breakpoint validation (#15754) ##debug

* Added RCoreBind.syncDebugMaps() and RCoreBind.getDebugMaps() api
* Refactor breakpoint validation ##debug
* Reenable db tests and add new tests to check validity
* Add perm check to isMapped and remove map sync to improve performance
This commit is contained in:
yossizap 2020-01-04 00:54:24 +00:00 committed by radare
parent d884f76e9f
commit 313d4b4893
9 changed files with 101 additions and 50 deletions

View File

@ -288,7 +288,7 @@ R_API int r_bp_list(RBreakpoint *bp, int rad) {
switch (rad) {
case 0:
bp->cb_printf ("0x%08"PFMT64x" - 0x%08"PFMT64x \
" %d %c%c%c %s %s %s cmd=\"%s\" cond=\"%s\" " \
" %d %c%c%c %s %s %s %s cmd=\"%s\" cond=\"%s\" " \
"name=\"%s\" module=\"%s\"\n",
b->addr, b->addr + b->size, b->size,
((b->perm & R_BP_PROT_READ) | (b->perm & R_BP_PROT_ACCESS)) ? 'r' : '-',
@ -297,6 +297,7 @@ R_API int r_bp_list(RBreakpoint *bp, int rad) {
b->hw ? "hw": "sw",
b->trace ? "trace" : "break",
b->enabled ? "enabled" : "disabled",
r_bp_is_valid (bp, b) ? "valid" : "invalid",
r_str_get2 (b->data),
r_str_get2 (b->cond),
r_str_get2 (b->name),
@ -319,7 +320,7 @@ R_API int r_bp_list(RBreakpoint *bp, int rad) {
bp->cb_printf ("%s{\"addr\":%"PFMT64d",\"size\":%d,"
"\"prot\":\"%c%c%c\",\"hw\":%s,"
"\"trace\":%s,\"enabled\":%s,"
"\"data\":\"%s\","
"\"valid\":%s,\"data\":\"%s\","
"\"cond\":\"%s\"}",
iter->p ? "," : "",
b->addr, b->size,
@ -329,6 +330,7 @@ R_API int r_bp_list(RBreakpoint *bp, int rad) {
b->hw ? "true" : "false",
b->trace ? "true" : "false",
b->enabled ? "true" : "false",
r_bp_is_valid (bp, b) ? "true" : "false",
r_str_get2 (b->data),
r_str_get2 (b->cond));
break;
@ -409,3 +411,12 @@ R_API int r_bp_size(RBreakpoint *bp) {
}
return bpsize;
}
// Check if the breakpoint is in a valid map
R_API bool r_bp_is_valid(RBreakpoint *bp, RBreakpointItem *b) {
if (!bp->bpinmaps) {
return true;
}
return bp->corebind.isMapped (bp->corebind.core, b->addr, b->perm);
}

View File

@ -38,6 +38,10 @@ R_API bool r_bp_restore_except(RBreakpoint *bp, bool set, ut64 addr) {
RListIter *iter;
RBreakpointItem *b;
if (set && bp->bpinmaps) {
bp->corebind.syncDebugMaps (bp->corebind.core);
}
r_list_foreach (bp->bps, iter, b) {
if (addr && b->addr == addr) {
continue;
@ -46,6 +50,10 @@ R_API bool r_bp_restore_except(RBreakpoint *bp, bool set, ut64 addr) {
if (set && !b->enabled) {
continue;
}
// Check if the breakpoint is in a valid map
if (set && bp->bpinmaps && !r_bp_is_valid (bp, b)) {
continue;
}
if (bp->breakpoint && bp->breakpoint (bp, b, set)) {
continue;
}

View File

@ -1329,6 +1329,13 @@ static bool cb_dbg_unlibs(void *user, void *data) {
return true;
}
static bool cb_dbg_bpinmaps(void *user, void *data) {
RCore *core = (RCore *) user;
RConfigNode *node = (RConfigNode *) data;
core->dbg->bp->bpinmaps = node->i_value;
return true;
}
static bool cb_dbg_forks(void *user, void *data) {
RCore *core = (RCore*) user;
RConfigNode *node = (RConfigNode*) data;
@ -3290,7 +3297,7 @@ R_API int r_core_config_init(RCore *core) {
SETPREF ("dbg.slow", "false", "Show stack and regs in visual mode in a slow but verbose mode");
SETPREF ("dbg.funcarg", "false", "Display arguments to function call in visual mode");
SETPREF ("dbg.bpinmaps", "true", "Force breakpoints to be inside a valid map");
SETCB ("dbg.bpinmaps", "true", &cb_dbg_bpinmaps, "Activate breakpoints only if they are inside a valid map");
SETCB ("dbg.forks", "false", &cb_dbg_forks, "Stop execution if fork() is done (see dbg.threads)");
n = NODECB ("dbg.btalgo", "fuzzy", &cb_dbg_btalgo);
SETDESC (n, "Select backtrace algorithm");

View File

@ -3023,23 +3023,6 @@ static void cmd_debug_reg(RCore *core, const char *str) {
}
}
static int validAddress(RCore *core, ut64 addr) {
RDebugMap *map;
RListIter *iter;
if (!r_config_get_i (core->config, "dbg.bpinmaps")) {
return core->num->value = 1;
}
r_debug_map_sync (core->dbg);
r_list_foreach (core->dbg->maps, iter, map) {
if (addr >= map->addr && addr < map->addr_end) {
return core->num->value = 1;
}
}
// TODO: try to read memory, expect no 0xffff
// TODO: check map permissions
return core->num->value = 0;
}
static void backtrace_vars(RCore *core, RList *frames) {
RDebugFrame *f;
RListIter *iter;
@ -3367,13 +3350,9 @@ static void r_core_cmd_bp(RCore *core, const char *input) {
case '.':
if (input[2]) {
ut64 addr = r_num_tail (core->num, core->offset, input + 2);
if (validAddress (core, addr)) {
bpi = r_debug_bp_add (core->dbg, addr, hwbp, false, 0, NULL, 0);
if (!bpi) {
eprintf ("Unable to add breakpoint (%s)\n", input + 2);
}
} else {
eprintf ("Invalid address\n");
bpi = r_debug_bp_add (core->dbg, addr, hwbp, false, 0, NULL, 0);
if (!bpi) {
eprintf ("Unable to add breakpoint (%s)\n", input + 2);
}
} else {
bpi = r_bp_get_at (core->dbg->bp, core->offset);
@ -3780,30 +3759,25 @@ static void r_core_cmd_bp(RCore *core, const char *input) {
}
}
addr = r_num_math (core->num, DB_ARG (i));
if (validAddress (core, addr)) {
bpi = r_debug_bp_add (core->dbg, addr, hwbp, watch, rw, NULL, 0);
if (bpi) {
free (bpi->name);
if (!strcmp (DB_ARG (i), "$$")) {
RFlagItem *f = r_core_flag_get_by_spaces (core->flags, addr);
if (f) {
if (addr > f->offset) {
bpi->name = r_str_newf ("%s+0x%" PFMT64x, f->name, addr - f->offset);
} else {
bpi->name = strdup (f->name);
}
bpi = r_debug_bp_add (core->dbg, addr, hwbp, watch, rw, NULL, 0);
if (bpi) {
free (bpi->name);
if (!strcmp (DB_ARG (i), "$$")) {
RFlagItem *f = r_core_flag_get_by_spaces (core->flags, addr);
if (f) {
if (addr > f->offset) {
bpi->name = r_str_newf ("%s+0x%" PFMT64x, f->name, addr - f->offset);
} else {
bpi->name = r_str_newf ("0x%08" PFMT64x, addr);
bpi->name = strdup (f->name);
}
} else {
bpi->name = strdup (DB_ARG (i));
bpi->name = r_str_newf ("0x%08" PFMT64x, addr);
}
} else {
eprintf ("Cannot set breakpoint at '%s'\n", DB_ARG (i));
bpi->name = strdup (DB_ARG (i));
}
} else {
eprintf ("Cannot place a breakpoint on 0x%08"PFMT64x" unmapped memory."
"See e? dbg.bpinmaps\n", addr);
eprintf ("Cannot set breakpoint at '%s'\n", DB_ARG (i));
}
}
}

View File

@ -274,14 +274,37 @@ static ut64 numget(RCore *core, const char *k) {
return r_num_math (core->num, k);
}
static bool __isMapped(RCore *core, ut64 addr) {
static bool __isMapped(RCore *core, ut64 addr, int perm) {
if (r_config_get_i (core->config, "cfg.debug")) {
r_debug_map_sync (core->dbg);
return r_debug_map_get (core->dbg, addr) != NULL;
RList *maps = core->dbg->maps;
RDebugMap *map = NULL;
RListIter *iter = NULL;
r_list_foreach (core->dbg->maps, iter, map) {
if (addr >= map->addr && addr < map->addr_end) {
if (perm > 0) {
if (map->perm & perm) {
return true;
}
} else {
return true;
}
}
}
return false;
}
return r_io_map_is_mapped (core->io, addr);
}
static int __syncDebugMaps(RCore *core) {
if (r_config_get_i (core->config, "cfg.debug")) {
return r_debug_map_sync (core->dbg);
}
return NULL;
}
R_API int r_core_bind(RCore *core, RCoreBind *bnd) {
bnd->core = core;
bnd->bphit = (RCoreDebugBpHit)r_core_debug_breakpoint_hit;
@ -299,6 +322,7 @@ R_API int r_core_bind(RCore *core, RCoreBind *bnd) {
bnd->cfgGet = (RCoreConfigGet)cfgget;
bnd->numGet = (RCoreNumGet)numget;
bnd->isMapped = (RCoreIsMapped)__isMapped;
bnd->syncDebugMaps = (RCoreDebugMapsSync)__syncDebugMaps;
return true;
}
@ -2710,6 +2734,7 @@ R_API bool r_core_init(RCore *core) {
r_io_bind (core->io, &(core->dbg->iob));
r_io_bind (core->io, &(core->dbg->bp->iob));
r_core_bind (core, &core->dbg->corebind);
r_core_bind (core, &core->dbg->bp->corebind);
core->dbg->anal = core->anal; // XXX: dupped instance.. can cause lost pointerz
//r_debug_use (core->dbg, "native");
// XXX pushing uninitialized regstate results in trashed reg values

View File

@ -14,7 +14,8 @@ typedef char* (*RCoreCmdStr)(void *core, const char *cmd);
typedef char* (*RCoreCmdStrF)(void *core, const char *cmd, ...);
typedef void (*RCorePuts)(const char *cmd);
typedef void (*RCoreSetArchBits)(void *core, const char *arch, int bits);
typedef bool (*RCoreIsMapped)(void *core, ut64 addr);
typedef bool (*RCoreIsMapped)(void *core, ut64 addr, int perm);
typedef int (*RCoreDebugMapsSync)(void *core);
typedef const char *(*RCoreGetName)(void *core, ut64 off);
typedef char *(*RCoreGetNameDelta)(void *core, ut64 off);
typedef void (*RCoreSeekArchBits)(void *core, ut64 addr);
@ -39,6 +40,7 @@ typedef struct r_core_bind_t {
RCoreConfigGet cfgGet;
RCoreNumGet numGet;
RCoreIsMapped isMapped;
RCoreDebugMapsSync syncDebugMaps;
} RCoreBind;
#endif

View File

@ -71,6 +71,8 @@ typedef struct r_bp_t {
int stepcont;
int endian;
int bits;
bool bpinmaps; /* Only enable breakpoints inside a valid map */
RCoreBind corebind;
RIOBind iob; // compile time dependency
RBreakpointPlugin *cur;
RList *traces; // XXX
@ -138,6 +140,8 @@ R_API RBreakpointItem *r_bp_item_new (RBreakpoint *bp);
R_API RBreakpointItem *r_bp_get_at (RBreakpoint *bp, ut64 addr);
R_API RBreakpointItem *r_bp_get_in (RBreakpoint *bp, ut64 addr, int perm);
R_API bool r_bp_is_valid(RBreakpoint *bp, RBreakpointItem *b);
R_API int r_bp_add_cond(RBreakpoint *bp, const char *cond);
R_API int r_bp_del_cond(RBreakpoint *bp, int idx);
R_API int r_bp_add_fault(RBreakpoint *bp, ut64 addr, int size, int perm);

View File

@ -195,3 +195,25 @@ Hello world!
31ed5e89
EOF
RUN
NAME='valid db'
FILE=../bins/elf/analysis/x86-helloworld-gcc
ARGS=-d
CMDS=<<EOF
db 0x0
db~1[12];
EXPECT=<<RUN
"invalid"
EOF
RUN
NAME='invalid db'
FILE=../bins/elf/analysis/x86-helloworld-gcc
ARGS=-d
CMDS=<<EOF
db entry0
db~1[12]
EXPECT=<<RUN
"valid"
EOF
RUN

View File

@ -1,7 +1,5 @@
NAME='Can set db on main'
FILE=../bins/elf/analysis/hello-objc-linux
# db output has changed
BROKEN=1
ARGS=-d
CMDS=<<EXPECT
db main;db~1[14];