From patchwork Fri Oct 19 20:39:03 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Liam Merwick X-Patchwork-Id: 10650165 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A86CE112B for ; Fri, 19 Oct 2018 20:47:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9476D2843B for ; Fri, 19 Oct 2018 20:47:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 822C32844C; Fri, 19 Oct 2018 20:47:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.7 required=2.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 048C52843B for ; Fri, 19 Oct 2018 20:47:28 +0000 (UTC) Received: from localhost ([::1]:52683 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gDbg8-0005mw-AH for patchwork-qemu-devel@patchwork.kernel.org; Fri, 19 Oct 2018 16:47:28 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54848) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gDbY4-0007zo-Mp for qemu-devel@nongnu.org; Fri, 19 Oct 2018 16:39:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gDbY3-0002im-Rj for qemu-devel@nongnu.org; Fri, 19 Oct 2018 16:39:08 -0400 Received: from aserp2120.oracle.com ([141.146.126.78]:48250) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gDbXx-0002YM-VA; Fri, 19 Oct 2018 16:39:02 -0400 Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1]) by aserp2120.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w9JKYQAI148280; Fri, 19 Oct 2018 20:39:01 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=from : to : cc : subject : date : message-id : in-reply-to : references; s=corp-2018-07-02; bh=00OlDj+T0V1H2/4mJ8UdvhdbNXJrHT/3fXCcsmSTYwk=; b=3Rkv7JPF0uYfksXdDH5dUlL0fyTMjuSa4m8dEzXCb8hWRjP1LPVXXlwda7z1XpR8OXrn +2pY9nqomLhcD6a+busDMu4gjzf7HVWrSVEmusiiNGOlm76sYGZArvaqDATyxi/tQ17l hjtC0RAcRo4DsvFN+Cbv4FY/sdo8nuA+JSwau7+f2rIj/k1SehVo09aeeKp+MnxosI68 Tg8w40Chdb9lOa/nKrGVtm7WBUNf12uhASBsdyyqR0GUCvc3UhfUkK86+itdoveCCPfd EThc1b/QXns11DHoFkIdm2UZEizEZPXZ5oJznJeIvZehwvPsWp/CXJqbD3VmHe27jBKY 7Q== Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by aserp2120.oracle.com with ESMTP id 2n38nqp1sy-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 19 Oct 2018 20:39:00 +0000 Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by aserv0021.oracle.com (8.14.4/8.14.4) with ESMTP id w9JKctNC027992 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 19 Oct 2018 20:38:55 GMT Received: from abhmp0004.oracle.com (abhmp0004.oracle.com [141.146.116.10]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id w9JKct7E024989; Fri, 19 Oct 2018 20:38:55 GMT Received: from ol7.uk.oracle.com (/10.175.186.240) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 19 Oct 2018 13:38:55 -0700 From: Liam Merwick To: qemu-devel@nongnu.org Date: Fri, 19 Oct 2018 21:39:03 +0100 Message-Id: <1539981546-10596-6-git-send-email-Liam.Merwick@oracle.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1539981546-10596-1-git-send-email-Liam.Merwick@oracle.com> References: <1539981546-10596-1-git-send-email-Liam.Merwick@oracle.com> X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=9051 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=4 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=394 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1810190183 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] X-Received-From: 141.146.126.78 Subject: [Qemu-devel] [PATCH v4 5/8] block: Fix potential Null pointer dereferences in vvfat.c X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, jsnow@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 --- block/vvfat.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index fc41841a5c3c..19f6725054a0 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -100,6 +100,7 @@ static inline void array_free(array_t* array) /* does not automatically grow */ static inline void* array_get(array_t* array,unsigned int index) { assert(index < array->next); + assert(array->pointer); return array->pointer + index * array->item_size; } @@ -108,8 +109,7 @@ static inline int array_ensure_allocated(array_t* array, int index) if((index + 1) * array->item_size > array->size) { int new_size = (index + 32) * array->item_size; array->pointer = g_realloc(array->pointer, new_size); - if (!array->pointer) - return -1; + assert(array->pointer); memset(array->pointer + array->size, 0, new_size - array->size); array->size = new_size; array->next = index + 1; @@ -2261,6 +2261,9 @@ static mapping_t* insert_mapping(BDRVVVFATState* s, } if (index >= s->mapping.next || mapping->begin > begin) { mapping = array_insert(&(s->mapping), index, 1); + if (mapping == NULL) { + return NULL; + } mapping->path = NULL; adjust_mapping_indices(s, index, +1); } @@ -2428,6 +2431,9 @@ static int commit_direntries(BDRVVVFATState* s, direntry_t* direntry = array_get(&(s->directory), dir_index); uint32_t first_cluster = dir_index == 0 ? 0 : begin_of_direntry(direntry); mapping_t* mapping = find_mapping_for_cluster(s, first_cluster); + if (mapping == NULL) { + return -1; + } int factor = 0x10 * s->sectors_per_cluster; int old_cluster_count, new_cluster_count; @@ -2494,6 +2500,9 @@ DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapp direntry = array_get(&(s->directory), first_dir_index + i); if (is_directory(direntry) && !is_dot(direntry)) { mapping = find_mapping_for_cluster(s, first_cluster); + if (mapping == NULL) { + return -1; + } assert(mapping->mode & MODE_DIRECTORY); ret = commit_direntries(s, first_dir_index + i, array_index(&(s->mapping), mapping)); @@ -2522,6 +2531,10 @@ static int commit_one_file(BDRVVVFATState* s, assert(offset < size); assert((offset % s->cluster_size) == 0); + if (mapping == NULL) { + return -1; + } + for (i = s->cluster_size; i < offset; i += s->cluster_size) c = modified_fat_get(s, c); @@ -2668,8 +2681,12 @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s) if (commit->action == ACTION_RENAME) { mapping_t* mapping = find_mapping_for_cluster(s, commit->param.rename.cluster); - char* old_path = mapping->path; + char *old_path; + if (mapping == NULL) { + return -1; + } + old_path = mapping->path; assert(commit->path); mapping->path = commit->path; if (rename(old_path, mapping->path)) @@ -2690,10 +2707,15 @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s) direntry_t* d = direntry + i; if (is_file(d) || (is_directory(d) && !is_dot(d))) { + int l; + char *new_path; mapping_t* m = find_mapping_for_cluster(s, begin_of_direntry(d)); - int l = strlen(m->path); - char* new_path = g_malloc(l + diff + 1); + if (m == NULL) { + return -1; + } + l = strlen(m->path); + new_path = g_malloc(l + diff + 1); assert(!strncmp(m->path, mapping->path, l2)); @@ -3193,6 +3215,7 @@ static int enable_write_target(BlockDriverState *bs, Error **errp) backing = bdrv_new_open_driver(&vvfat_write_target, NULL, BDRV_O_ALLOW_RDWR, &error_abort); + assert(backing); *(void**) backing->opaque = s; bdrv_set_backing_hd(s->bs, backing, &error_abort);