From patchwork Mon Nov 5 21:38:38 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Liam Merwick X-Patchwork-Id: 10669259 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 441371803 for ; Mon, 5 Nov 2018 21:43:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3E06B29E63 for ; Mon, 5 Nov 2018 21:43:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 322F829E79; Mon, 5 Nov 2018 21:43:09 +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 97BE429E63 for ; Mon, 5 Nov 2018 21:43:08 +0000 (UTC) Received: from localhost ([::1]:37714 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gJmeJ-0003hV-LR for patchwork-qemu-devel@patchwork.kernel.org; Mon, 05 Nov 2018 16:43:07 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36166) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gJmaL-0007ET-1s for qemu-devel@nongnu.org; Mon, 05 Nov 2018 16:39:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gJmaH-0003Af-AR for qemu-devel@nongnu.org; Mon, 05 Nov 2018 16:39:00 -0500 Received: from aserp2120.oracle.com ([141.146.126.78]:33180) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gJmZy-0002cv-AL; Mon, 05 Nov 2018 16:38:40 -0500 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 wA5LT3eb151003; Mon, 5 Nov 2018 21:38:31 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=HKoibhI/5sD5PGNfIGsaQG6RgX324uDfu8RirhPc6xI=; b=Yfq0l5robuOx453kYrr3hSDGvxL1npBdgH5dwLQKEqnDD9wRgorew24ekvQsuMWntVKT N1AliL9j8IfZu0fHh6arNSx4Y6fFzd4Ve5oOxMX5z0bP47rMUrwJ11YVYp8vNMGLgnDS L18tkBQWSMQQYKph4Ww99WqxqdXCvV3Ii+lKN1N/PQunpKAu0vf25FiM3yEJ3yLJ2c2J 9SYGXmlpjPfShT7/3LnZajy6KZ7YwyUEvjIsfsXzMZoT4jBRCzUXUAUD206NcXaVfyDv plumHbCCVTWV4n6kIGRixuP7EtyjMdqVmI/BB67oE+mJ47zwxQLajiRlEXsY/4aCobBy ig== Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by aserp2120.oracle.com with ESMTP id 2nh3mphpy9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 05 Nov 2018 21:38:31 +0000 Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by aserv0021.oracle.com (8.14.4/8.14.4) with ESMTP id wA5LcUZQ027402 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 5 Nov 2018 21:38:30 GMT Received: from abhmp0006.oracle.com (abhmp0006.oracle.com [141.146.116.12]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id wA5LcTVq000430; Mon, 5 Nov 2018 21:38:30 GMT Received: from ol7.uk.oracle.com (/10.175.201.67) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 05 Nov 2018 13:38:29 -0800 From: Liam Merwick To: qemu-devel@nongnu.org Date: Mon, 5 Nov 2018 21:38:38 +0000 Message-Id: <1541453919-25973-5-git-send-email-Liam.Merwick@oracle.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1541453919-25973-1-git-send-email-Liam.Merwick@oracle.com> References: <1541453919-25973-1-git-send-email-Liam.Merwick@oracle.com> X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=9068 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=3 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=769 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1811050190 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] X-Received-From: 141.146.126.78 Subject: [Qemu-devel] [PATCH v5 4/5] 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 Reviewed-by: Max Reitz --- block/vvfat.c | 50 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index fc41841a5c3c..263274d9739a 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -100,30 +100,26 @@ 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; } -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) { 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; } - - return 0; } static inline void* array_get_next(array_t* array) { unsigned int next = array->next; - if (array_ensure_allocated(array, next) < 0) - return NULL; - + array_ensure_allocated(array, next); array->next = next + 1; return array_get(array, next); } @@ -2428,16 +2424,13 @@ 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); - int factor = 0x10 * s->sectors_per_cluster; int old_cluster_count, new_cluster_count; - int current_dir_index = mapping->info.dir.first_dir_index; - int first_dir_index = current_dir_index; + int current_dir_index; + int first_dir_index; int ret, i; uint32_t c; -DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapping->path, parent_mapping_index)); - assert(direntry); assert(mapping); assert(mapping->begin == first_cluster); @@ -2445,6 +2438,15 @@ DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapp assert(mapping->mode & MODE_DIRECTORY); assert(dir_index == 0 || is_directory(direntry)); + if (mapping == NULL) { + return -1; + } + +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; if (first_cluster == 0) { @@ -2494,6 +2496,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 +2527,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 +2677,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 +2703,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));