A bit of refactoring in RBin (#15349)

* introduce r_bin_open_buf
* fix r_bin_file_delete to use bf->id and not fd. This was used
  inconsistently and the function was just wrong.
* rewrite r_bin_reload to just reuse the existing bf->buf
* fix some memory leaks
This commit is contained in:
Riccardo Schirone 2019-10-24 14:33:54 +02:00 committed by radare
parent ce14b35ca5
commit 7594b0f5e6
9 changed files with 89 additions and 135 deletions

View File

@ -545,19 +545,20 @@ R_API ut64 r_bin_file_delete_all(RBin *bin) {
return 0;
}
R_API bool r_bin_file_delete(RBin *bin, ut32 bin_fd) {
R_API bool r_bin_file_delete(RBin *bin, ut32 bin_id) {
r_return_val_if_fail (bin, false);
RListIter *iter;
RBinFile *bf = r_bin_cur (bin);
if (bin && bf) {
r_list_foreach (bin->binfiles, iter, bf) {
if (bf && bf->fd == bin_fd) {
if (bf->fd == bin_fd) {
//avoiding UaF due to dead reference
bin->cur = NULL;
}
r_list_delete (bin->binfiles, iter);
return true;
RBinFile *bf, *cur = r_bin_cur (bin);
r_list_foreach (bin->binfiles, iter, bf) {
if (bf && bf->id == bin_id) {
if (cur && cur->id == bin_id) {
// avoiding UaF due to dead reference
bin->cur = NULL;
}
r_list_delete (bin->binfiles, iter);
return true;
}
}
return false;

View File

@ -228,64 +228,71 @@ R_API bool r_bin_open(RBin *bin, const char *file, RBinOptions *opt) {
return r_bin_open_io (bin, opt);
}
// XXX this function is full of mess, shuold be rewritten after the refactorings
R_API bool r_bin_reload(RBin *bin, int fd, ut64 baseaddr) {
RIOBind *iob = &(bin->iob);
R_API bool r_bin_reload(RBin *bin, ut32 bf_id, ut64 baseaddr) {
r_return_val_if_fail (bin, false);
r_return_val_if_fail (bin && iob && iob->io, false);
const char *name = iob->fd_get_name (iob->io, fd);
RBinFile *bf = r_bin_file_find_by_name (bin, name);
RBinFile *bf = r_bin_file_find_by_id (bin, bf_id);
if (!bf) {
eprintf ("r_bin_reload: No file to reopen\n");
return false;
}
RBinOptions opt;
r_bin_options_init (&opt, fd, baseaddr, bf->loadaddr, bin->rawstr);
r_bin_options_init (&opt, bf->fd, baseaddr, bf->loadaddr, bin->rawstr);
opt.filename = bf->file;
// invalidate current object reference
bf->o = NULL;
ut64 sz = iob->fd_size (iob->io, fd);
if (sz == UT64_MAX) {
sz = 128 * 1024;
bool res = r_bin_open_buf (bin, bf->buf, &opt);
r_bin_file_delete (bin, bf->id);
return res;
}
R_API bool r_bin_open_buf(RBin *bin, RBuffer *buf, RBinOptions *opt) {
r_return_val_if_fail (bin && opt, false);
r_return_val_if_fail ((st64)opt->sz >= 0, false);
RListIter *it;
RBinXtrPlugin *xtr;
bin->rawstr = opt->rawstr;
bin->file = opt->filename;
if (opt->loadaddr == UT64_MAX) {
opt->loadaddr = 0;
}
// TODO: deprecate, the code in the else should be enough
if (sz == UT64_MAX) {
if (!iob->fd_is_dbg (iob->io, fd)) {
// too big, probably wrong
eprintf ("Warning: file is too big and not in debugger\n");
if (!opt->sz) {
opt->sz = r_buf_size (buf);
}
RBinFile *bf = NULL;
if (bin->use_xtr && !opt->pluginname) {
// XXX - for the time being this is fine, but we may want to
// change the name to something like
// <xtr_name>:<bin_type_name>
r_list_foreach (bin->binxtrs, it, xtr) {
if (!xtr->check_buffer) {
eprintf ("Missing check_buffer callback for '%s'\n", xtr->name);
continue;
}
if (xtr->check_buffer (buf)) {
if (xtr->extract_from_buffer || xtr->extractall_from_buffer ||
xtr->extract_from_bytes || xtr->extractall_from_bytes) {
bf = r_bin_file_xtr_load_buffer (bin, xtr,
bin->file, buf, opt->baseaddr, opt->loadaddr,
opt->xtr_idx, opt->fd, bin->rawstr);
}
}
}
}
if (!bf) {
bf = r_bin_file_new_from_buffer (bin, bin->file, buf, bin->rawstr,
opt->baseaddr, opt->loadaddr, opt->fd, opt->pluginname);
if (!bf) {
return false;
}
// attempt a local open and read
// This happens when a plugin like debugger does not have a
// fixed size.
// if there is no fixed size or its MAXED, there is no way to
// definitively
// load the bin-properly. Many of the plugins require all
// content and are not
// stream based loaders
int tfd = iob->fd_open (iob->io, name, R_PERM_R, 0);
if (tfd < 0) {
return false;
}
sz = iob->fd_size (iob->io, tfd);
if (sz == UT64_MAX) {
iob->fd_close (iob->io, tfd);
return false;
}
iob->fd_close (iob->io, tfd);
}
if (!r_bin_file_set_cur_binfile (bin, bf)) {
return false;
}
bool res = false;
ut8 *buf_bytes = calloc (1, sz + 1);
if (buf_bytes) {
if (iob->fd_read_at (iob->io, fd, 0LL, buf_bytes, sz)) {
r_bin_file_set_bytes (bf, buf_bytes, sz, false);
res = true;
}
free (buf_bytes);
}
return res && r_bin_open_io (bin, &opt);
r_id_storage_set (bin->ids, bin->cur, bf->id);
return true;
}
R_API bool r_bin_open_io(RBin *bin, RBinOptions *opt) {
@ -294,13 +301,9 @@ R_API bool r_bin_open_io(RBin *bin, RBinOptions *opt) {
RIOBind *iob = &(bin->iob);
RIO *io = iob? iob->io: NULL;
RListIter *it;
RBinXtrPlugin *xtr;
bool is_debugger = iob->fd_is_dbg (io, opt->fd);
const char *fname = iob->fd_get_name (io, opt->fd);
bin->rawstr = opt->rawstr;
bin->file = fname;
if (opt->loadaddr == UT64_MAX) {
opt->loadaddr = 0;
}
@ -338,38 +341,10 @@ R_API bool r_bin_open_io(RBin *bin, RBinOptions *opt) {
buf = slice;
}
RBinFile *bf = NULL;
if (bin->use_xtr && !opt->pluginname) {
// XXX - for the time being this is fine, but we may want to
// change the name to something like
// <xtr_name>:<bin_type_name>
r_list_foreach (bin->binxtrs, it, xtr) {
if (!xtr->check_buffer) {
eprintf ("Missing check_buffer callback for '%s'\n", xtr->name);
continue;
}
if (xtr->check_buffer (buf)) {
if (xtr->extract_from_buffer || xtr->extractall_from_buffer ||
xtr->extract_from_bytes || xtr->extractall_from_bytes) {
bf = r_bin_file_xtr_load_buffer (bin, xtr,
fname, buf, opt->baseaddr, opt->loadaddr,
opt->xtr_idx, opt->fd, bin->rawstr);
}
}
}
}
if (!bf) {
bf = r_bin_file_new_from_buffer (bin, fname, buf, bin->rawstr,
opt->baseaddr, opt->loadaddr, opt->fd, opt->pluginname);
if (!bf) {
return false;
}
}
if (!r_bin_file_set_cur_binfile (bin, bf)) {
return false;
}
r_id_storage_set (bin->ids, bin->cur, bf->id);
return true;
opt->filename = fname;
bool res = r_bin_open_buf (bin, buf, opt);
r_buf_free (buf);
return res;
}
R_IPI RBinPlugin *r_bin_get_binplugin_by_name(RBin *bin, const char *name) {

View File

@ -335,8 +335,7 @@ R_API int r_bin_object_set_items(RBinFile *bf, RBinObject *o) {
REBASE_PADDR (o, l, RBinReloc);
o->relocs = list2rbtree (l);
l->free = NULL;
// causes double free
//r_list_free (l);
r_list_free (l);
}
}
}

View File

@ -1662,12 +1662,11 @@ R_API RList *r_bin_dwarf_parse_line(RBin *a, int mode) {
free (buf);
return NULL;
}
list = r_list_new (); // always return empty list wtf
list = r_list_newf (r_bin_dwarf_row_free); // always return empty list wtf
if (!list) {
free (buf);
return NULL;
}
list->free = r_bin_dwarf_row_free;
r_bin_dwarf_parse_line_raw2 (a, buf, len, mode);
// k bin/cur/addrinfo/*
SdbListIter *iter;

View File

@ -998,8 +998,7 @@ static int bin_dwarf(RCore *core, int mode) {
r_cons_break_pop ();
R_FREE (lastFileContents);
R_FREE (lastFileContents2);
// this list is owned by rbin, not us, we shouldn't free it
// r_list_free (list);
r_list_free (list);
free (lastFileLines);
return true;
}
@ -2377,6 +2376,7 @@ static bool io_create_mem_map(RIO *io, RBinSection *sec, ut64 at) {
return false;
}
// let the section refere to the map as a memory-map
free (map->name);
map->name = r_str_newf ("mmap.%s", sec->name);
return true;
}
@ -3936,16 +3936,6 @@ R_API bool r_core_bin_delete(RCore *core, ut32 bf_id) {
if (bf_id == UT32_MAX) {
return false;
}
#if 0
if (!r_bin_object_delete (core->bin, bf_id)) {
return false;
}
// TODO: use rbinat()
RBinFile *bf = r_bin_cur (core->bin);
if (bf) {
r_io_use_fd (core->io, bf->fd);
}
#endif
r_bin_file_delete (core->bin, bf_id);
RBinFile *bf = r_bin_file_at (core->bin, core->offset);
if (bf) {

View File

@ -117,8 +117,8 @@ R_API int r_core_file_reopen(RCore *core, const char *args, int perm, int loadbi
if (file) {
bool had_rbin_info = false;
if (ofile) {
if (r_bin_file_delete (core->bin, ofile->fd)) {
if (ofile && bf) {
if (r_bin_file_delete (core->bin, bf->id)) {
had_rbin_info = true;
}
}

View File

@ -408,7 +408,8 @@ static int __r_core_bin_reload(RCore *r, const char *file, ut64 baseaddr) {
int result = 0;
RCoreFile *cf = r_core_file_cur (r);
if (cf) {
result = r_bin_reload (r->bin, cf->fd, baseaddr);
RBinFile *bf = r_bin_file_find_by_fd (r->bin, cf->fd);
result = r_bin_reload (r->bin, bf->id, baseaddr);
}
r_core_bin_set_env (r, r_bin_cur (r->bin));
return result;

View File

@ -158,17 +158,6 @@ static const char *help_msg_oonn[] = {
NULL
};
static RBinFile *find_binfile_by_id(RBin *bin, ut32 id) {
RListIter *it;
RBinFile *bf;
r_list_foreach (bin->binfiles, it, bf) {
if (bf->id == id) {
return bf;
}
}
return NULL;
}
static void cmd_open_init(RCore *core) {
DEFINE_CMD_DESCRIPTOR (core, o);
DEFINE_CMD_DESCRIPTOR_SPECIAL (core, o*, o_star);
@ -297,7 +286,7 @@ static void cmd_open_bin(RCore *core, const char *input) {
break;
case ' ': // "ob "
{
ut32 fd;
ut32 id;
int n;
const char *tmp;
char *v;
@ -313,12 +302,12 @@ static void cmd_open_bin(RCore *core, const char *input) {
break;
}
tmp = r_str_word_get0 (v, 0);
fd = *v && r_is_valid_input_num_value (core->num, tmp)
id = *v && r_is_valid_input_num_value (core->num, tmp)
? r_get_input_num_value (core->num, tmp): UT32_MAX;
if (n == 2) {
tmp = r_str_word_get0 (v, 1);
} else {
binfile_num = fd;
binfile_num = id;
}
r_core_bin_raise (core, binfile_num);
free (v);
@ -351,23 +340,21 @@ static void cmd_open_bin(RCore *core, const char *input) {
if (input[2] == '*') {
r_bin_file_delete_all (core->bin);
} else {
ut32 fd;
ut32 id;
value = r_str_trim_ro (input + 2);
if (!value) {
eprintf ("Invalid argument\n");
break;
}
fd = (*value && r_is_valid_input_num_value (core->num, value)) ?
id = (*value && r_is_valid_input_num_value (core->num, value)) ?
r_get_input_num_value (core->num, value) : UT32_MAX;
RBinFile *bf = find_binfile_by_id (core->bin, fd);
RBinFile *bf = r_bin_file_find_by_id (core->bin, id);
if (!bf) {
eprintf ("Invalid binid\n");
break;
}
if (r_core_bin_delete (core, bf->id)) {
if (!r_bin_file_delete (core->bin, fd)) {
eprintf ("Cannot find an RBinFile associated with that fd.\n");
}
if (!r_core_bin_delete (core, bf->id)) {
eprintf ("Cannot find an RBinFile associated with that id.\n");
}
}
break;

View File

@ -649,6 +649,7 @@ typedef struct r_bin_options_t {
int xtr_idx; // load Nth binary
int rawstr;
int fd;
const char *filename;
} RBinOptions;
R_API RBinImport *r_bin_import_clone(RBinImport *o);
@ -664,7 +665,8 @@ R_API RBin *r_bin_new(void);
R_API void r_bin_free(RBin *bin);
R_API bool r_bin_open(RBin *bin, const char *file, RBinOptions *opt);
R_API bool r_bin_open_io(RBin *bin, RBinOptions *opt);
R_API bool r_bin_reload(RBin *bin, int fd, ut64 baseaddr);
R_API bool r_bin_open_buf(RBin *bin, RBuffer *buf, RBinOptions *opt);
R_API bool r_bin_reload(RBin *bin, ut32 bf_id, ut64 baseaddr);
// plugins/bind functions
R_API void r_bin_bind(RBin *b, RBinBind *bnd);
@ -756,7 +758,7 @@ R_API bool r_bin_file_set_cur_by_fd(RBin *bin, ut32 bin_fd);
R_API bool r_bin_file_set_cur_by_id(RBin *bin, ut32 bin_id);
R_API bool r_bin_file_set_cur_by_name(RBin *bin, const char *name);
R_API ut64 r_bin_file_delete_all(RBin *bin);
R_API bool r_bin_file_delete(RBin *bin, ut32 bin_fd);
R_API bool r_bin_file_delete(RBin *bin, ut32 bin_id);
R_API bool r_bin_file_hash(RBin *bin, ut64 limit, const char *file, RList/*<RBinFileHash>*/ **old_file_hashes);
R_API RBinPlugin *r_bin_file_cur_plugin(RBinFile *binfile);
R_API void r_bin_file_hash_free(RBinFileHash *fhash);