block: Fix potential Null pointer dereferences in vvfat.c

The calls to find_mapping_for_cluster() may return NULL but it
isn't always checked for before dereferencing the value returned.
Additionally, add some asserts to cover cases where NULL can't
be returned but which might not be obvious at first glance.

Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
Message-id: 1541453919-25973-5-git-send-email-Liam.Merwick@oracle.com
[mreitz: Dropped superfluous check of "mapping" following an assertion
         that it is not NULL, and fixed some indentation]
Signed-off-by: Max Reitz <mreitz@redhat.com>
This commit is contained in:
Liam Merwick 2018-11-05 21:38:38 +00:00 committed by Max Reitz
parent 2e2db26009
commit 8d9401c279

View file

@ -100,30 +100,26 @@ static inline void array_free(array_t* array)
/* does not automatically grow */ /* does not automatically grow */
static inline void* array_get(array_t* array,unsigned int index) { static inline void* array_get(array_t* array,unsigned int index) {
assert(index < array->next); assert(index < array->next);
assert(array->pointer);
return array->pointer + index * array->item_size; return array->pointer + index * array->item_size;
} }
static inline int array_ensure_allocated(array_t* array, int index) static inline void array_ensure_allocated(array_t *array, int index)
{ {
if((index + 1) * array->item_size > array->size) { if((index + 1) * array->item_size > array->size) {
int new_size = (index + 32) * array->item_size; int new_size = (index + 32) * array->item_size;
array->pointer = g_realloc(array->pointer, new_size); array->pointer = g_realloc(array->pointer, new_size);
if (!array->pointer) assert(array->pointer);
return -1;
memset(array->pointer + array->size, 0, new_size - array->size); memset(array->pointer + array->size, 0, new_size - array->size);
array->size = new_size; array->size = new_size;
array->next = index + 1; array->next = index + 1;
} }
return 0;
} }
static inline void* array_get_next(array_t* array) { static inline void* array_get_next(array_t* array) {
unsigned int next = array->next; unsigned int next = array->next;
if (array_ensure_allocated(array, next) < 0) array_ensure_allocated(array, next);
return NULL;
array->next = next + 1; array->next = next + 1;
return array_get(array, next); return array_get(array, next);
} }
@ -2422,16 +2418,13 @@ static int commit_direntries(BDRVVVFATState* s,
direntry_t* direntry = array_get(&(s->directory), dir_index); direntry_t* direntry = array_get(&(s->directory), dir_index);
uint32_t first_cluster = dir_index == 0 ? 0 : begin_of_direntry(direntry); uint32_t first_cluster = dir_index == 0 ? 0 : begin_of_direntry(direntry);
mapping_t* mapping = find_mapping_for_cluster(s, first_cluster); mapping_t* mapping = find_mapping_for_cluster(s, first_cluster);
int factor = 0x10 * s->sectors_per_cluster; int factor = 0x10 * s->sectors_per_cluster;
int old_cluster_count, new_cluster_count; int old_cluster_count, new_cluster_count;
int current_dir_index = mapping->info.dir.first_dir_index; int current_dir_index;
int first_dir_index = current_dir_index; int first_dir_index;
int ret, i; int ret, i;
uint32_t c; uint32_t c;
DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapping->path, parent_mapping_index));
assert(direntry); assert(direntry);
assert(mapping); assert(mapping);
assert(mapping->begin == first_cluster); assert(mapping->begin == first_cluster);
@ -2439,6 +2432,11 @@ DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapp
assert(mapping->mode & MODE_DIRECTORY); assert(mapping->mode & MODE_DIRECTORY);
assert(dir_index == 0 || is_directory(direntry)); assert(dir_index == 0 || is_directory(direntry));
DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n",
mapping->path, parent_mapping_index));
current_dir_index = mapping->info.dir.first_dir_index;
first_dir_index = current_dir_index;
mapping->info.dir.parent_mapping_index = parent_mapping_index; mapping->info.dir.parent_mapping_index = parent_mapping_index;
if (first_cluster == 0) { if (first_cluster == 0) {
@ -2488,6 +2486,9 @@ DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapp
direntry = array_get(&(s->directory), first_dir_index + i); direntry = array_get(&(s->directory), first_dir_index + i);
if (is_directory(direntry) && !is_dot(direntry)) { if (is_directory(direntry) && !is_dot(direntry)) {
mapping = find_mapping_for_cluster(s, first_cluster); mapping = find_mapping_for_cluster(s, first_cluster);
if (mapping == NULL) {
return -1;
}
assert(mapping->mode & MODE_DIRECTORY); assert(mapping->mode & MODE_DIRECTORY);
ret = commit_direntries(s, first_dir_index + i, ret = commit_direntries(s, first_dir_index + i,
array_index(&(s->mapping), mapping)); array_index(&(s->mapping), mapping));
@ -2516,6 +2517,10 @@ static int commit_one_file(BDRVVVFATState* s,
assert(offset < size); assert(offset < size);
assert((offset % s->cluster_size) == 0); assert((offset % s->cluster_size) == 0);
if (mapping == NULL) {
return -1;
}
for (i = s->cluster_size; i < offset; i += s->cluster_size) for (i = s->cluster_size; i < offset; i += s->cluster_size)
c = modified_fat_get(s, c); c = modified_fat_get(s, c);
@ -2662,8 +2667,12 @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s)
if (commit->action == ACTION_RENAME) { if (commit->action == ACTION_RENAME) {
mapping_t* mapping = find_mapping_for_cluster(s, mapping_t* mapping = find_mapping_for_cluster(s,
commit->param.rename.cluster); commit->param.rename.cluster);
char* old_path = mapping->path; char *old_path;
if (mapping == NULL) {
return -1;
}
old_path = mapping->path;
assert(commit->path); assert(commit->path);
mapping->path = commit->path; mapping->path = commit->path;
if (rename(old_path, mapping->path)) if (rename(old_path, mapping->path))
@ -2684,10 +2693,15 @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s)
direntry_t* d = direntry + i; direntry_t* d = direntry + i;
if (is_file(d) || (is_directory(d) && !is_dot(d))) { if (is_file(d) || (is_directory(d) && !is_dot(d))) {
int l;
char *new_path;
mapping_t* m = find_mapping_for_cluster(s, mapping_t* m = find_mapping_for_cluster(s,
begin_of_direntry(d)); begin_of_direntry(d));
int l = strlen(m->path); if (m == NULL) {
char* new_path = g_malloc(l + diff + 1); return -1;
}
l = strlen(m->path);
new_path = g_malloc(l + diff + 1);
assert(!strncmp(m->path, mapping->path, l2)); assert(!strncmp(m->path, mapping->path, l2));