From patchwork Wed Aug 19 16:28:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joao Marcos Costa X-Patchwork-Id: 1347928 X-Patchwork-Delegate: trini@ti.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.denx.de (client-ip=2a01:238:438b:c500:173d:9f52:ddab:ee01; helo=phobos.denx.de; envelope-from=u-boot-bounces@lists.denx.de; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=bootlin.com Received: from phobos.denx.de (phobos.denx.de [IPv6:2a01:238:438b:c500:173d:9f52:ddab:ee01]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4BWtX20lFjz9sPf for ; Thu, 20 Aug 2020 02:29:05 +1000 (AEST) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 6BE738220C; Wed, 19 Aug 2020 18:28:49 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=bootlin.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id D6DED81F93; Wed, 19 Aug 2020 18:28:45 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: * X-Spam-Status: No, score=1.4 required=5.0 tests=BAYES_00,RCVD_IN_MSPIKE_H2, RCVD_IN_SBL_CSS,SPF_HELO_NONE,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 2873881E39 for ; Wed, 19 Aug 2020 18:28:43 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=bootlin.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=joaomarcos.costa@bootlin.com X-Originating-IP: 46.193.64.106 Received: from localhost.localdomain (eth-east-parth2-46-193-64-106.wb.wifirst.net [46.193.64.106]) (Authenticated sender: joaomarcos.costa@bootlin.com) by relay9-d.mail.gandi.net (Postfix) with ESMTPA id 714D3FF810; Wed, 19 Aug 2020 16:28:42 +0000 (UTC) From: Joao Marcos Costa To: u-boot@lists.denx.de Cc: joaomarcos.costa@bootlin.com Subject: [PATCH 1/1] fs/squashfs: Fix Coverity Scan defects Date: Wed, 19 Aug 2020 18:28:41 +0200 Message-Id: <20200819162841.23955-2-joaomarcos.costa@bootlin.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200819162841.23955-1-joaomarcos.costa@bootlin.com> References: <20200819162841.23955-1-joaomarcos.costa@bootlin.com> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.102.3 at phobos.denx.de X-Virus-Status: Clean Fix defects such as uninitialized variables and untrusted pointer operations. Most part of the tainted variables and the related defects actually comes from Linux's macro get_unaligned_le**, extensively used in SquashFS code. Add sanity checks for those variables. Signed-off-by: Joao Marcos Costa --- fs/squashfs/sqfs.c | 40 +++++++++++++++++++++++++++++----------- fs/squashfs/sqfs_dir.c | 3 +++ fs/squashfs/sqfs_inode.c | 5 ++++- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c index 9bd7b98d88..f67f7c4a40 100644 --- a/fs/squashfs/sqfs.c +++ b/fs/squashfs/sqfs.c @@ -154,6 +154,11 @@ static int sqfs_frag_lookup(u32 inode_fragment_index, header = get_unaligned_le16(metadata_buffer + table_offset); metadata = metadata_buffer + table_offset + SQFS_HEADER_SIZE; + if (!metadata) { + ret = -ENOMEM; + goto free_buffer; + } + entries = malloc(SQFS_METADATA_BLOCK_SIZE); if (!entries) { ret = -ENOMEM; @@ -272,8 +277,8 @@ static int sqfs_join(char **strings, char *dest, int start, int end, */ static int sqfs_tokenize(char **tokens, int count, const char *str) { + int i, j, ret = 0; char *aux, *strc; - int i, j; strc = strdup(str); if (!strc) @@ -282,8 +287,8 @@ static int sqfs_tokenize(char **tokens, int count, const char *str) if (!strcmp(strc, "/")) { tokens[0] = strdup(strc); if (!tokens[0]) { - free(strc); - return -ENOMEM; + ret = -ENOMEM; + goto free_strc; } } else { for (j = 0; j < count; j++) { @@ -292,15 +297,16 @@ static int sqfs_tokenize(char **tokens, int count, const char *str) if (!tokens[j]) { for (i = 0; i < j; i++) free(tokens[i]); - free(strc); - return -ENOMEM; + ret = -ENOMEM; + goto free_strc; } } } +free_strc: free(strc); - return 0; + return ret; } /* @@ -428,9 +434,9 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list, { struct squashfs_super_block *sblk = ctxt.sblk; char *path, *target, **sym_tokens, *res, *rem; + struct squashfs_ldir_inode *ldir = NULL; int j, ret, new_inode_number, offset; struct squashfs_symlink_inode *sym; - struct squashfs_ldir_inode *ldir; struct squashfs_dir_inode *dir; struct fs_dir_stream *dirsp; struct fs_dirent *dent; @@ -630,7 +636,7 @@ static int sqfs_read_inode_table(unsigned char **inode_table) int j, ret = 0, metablks_count; unsigned char *src_table, *itb; u32 src_len, dest_offset = 0; - unsigned long dest_len; + unsigned long dest_len = 0; bool compressed; table_size = get_unaligned_le64(&sblk->directory_table_start) - @@ -685,6 +691,7 @@ static int sqfs_read_inode_table(unsigned char **inode_table) goto free_itb; } + dest_offset += dest_len; } else { memcpy(*inode_table + (j * SQFS_METADATA_BLOCK_SIZE), src_table, src_len); @@ -694,7 +701,7 @@ static int sqfs_read_inode_table(unsigned char **inode_table) * Offsets to the decompression destination, to the metadata * buffer 'itb' and to the decompression source, respectively. */ - dest_offset += dest_len; + table_offset += src_len + SQFS_HEADER_SIZE; src_table += src_len + SQFS_HEADER_SIZE; } @@ -712,7 +719,7 @@ static int sqfs_read_directory_table(unsigned char **dir_table, u32 **pos_list) int j, ret = 0, metablks_count = -1; unsigned char *src_table, *dtb; u32 src_len, dest_offset = 0; - unsigned long dest_len; + unsigned long dest_len = 0; bool compressed; /* DIRECTORY TABLE */ @@ -781,6 +788,8 @@ static int sqfs_read_directory_table(unsigned char **dir_table, u32 **pos_list) dest_offset += dest_len; break; } + + dest_offset += dest_len; } else { memcpy(*dir_table + (j * SQFS_METADATA_BLOCK_SIZE), src_table, src_len); @@ -790,7 +799,6 @@ static int sqfs_read_directory_table(unsigned char **dir_table, u32 **pos_list) * Offsets to the decompression destination, to the metadata * buffer 'dtb' and to the decompression source, respectively. */ - dest_offset += dest_len; table_offset += src_len + SQFS_HEADER_SIZE; src_table += src_len + SQFS_HEADER_SIZE; } @@ -1138,6 +1146,9 @@ static int sqfs_get_regfile_info(struct squashfs_reg_inode *reg, finfo->start = get_unaligned_le32(®->start_block); finfo->frag = SQFS_IS_FRAGMENTED(get_unaligned_le32(®->fragment)); + if (finfo->size < 1 || finfo->offset < 0 || finfo->start < 0) + return -EINVAL; + if (finfo->frag) { datablk_count = finfo->size / le32_to_cpu(blksz); ret = sqfs_frag_lookup(get_unaligned_le32(®->fragment), @@ -1145,6 +1156,8 @@ static int sqfs_get_regfile_info(struct squashfs_reg_inode *reg, if (ret < 0) return -EINVAL; finfo->comp = true; + if (fentry->size < 1 || fentry->start < 0) + return -EINVAL; } else { datablk_count = DIV_ROUND_UP(finfo->size, le32_to_cpu(blksz)); } @@ -1168,6 +1181,9 @@ static int sqfs_get_lregfile_info(struct squashfs_lreg_inode *lreg, finfo->start = get_unaligned_le64(&lreg->start_block); finfo->frag = SQFS_IS_FRAGMENTED(get_unaligned_le32(&lreg->fragment)); + if (finfo->size < 1 || finfo->offset < 0 || finfo->start < 0) + return -EINVAL; + if (finfo->frag) { datablk_count = finfo->size / le32_to_cpu(blksz); ret = sqfs_frag_lookup(get_unaligned_le32(&lreg->fragment), @@ -1175,6 +1191,8 @@ static int sqfs_get_lregfile_info(struct squashfs_lreg_inode *lreg, if (ret < 0) return -EINVAL; finfo->comp = true; + if (fentry->size < 1 || fentry->start < 0) + return -EINVAL; } else { datablk_count = DIV_ROUND_UP(finfo->size, le32_to_cpu(blksz)); } diff --git a/fs/squashfs/sqfs_dir.c b/fs/squashfs/sqfs_dir.c index 5f7660aeae..00d2891e7d 100644 --- a/fs/squashfs/sqfs_dir.c +++ b/fs/squashfs/sqfs_dir.c @@ -53,6 +53,9 @@ int sqfs_dir_offset(void *dir_i, u32 *m_list, int m_count) return -EINVAL; } + if (offset < 0) + return -EINVAL; + for (j = 0; j < m_count; j++) { if (m_list[j] == start_block) return (++j * SQFS_METADATA_BLOCK_SIZE) + offset; diff --git a/fs/squashfs/sqfs_inode.c b/fs/squashfs/sqfs_inode.c index b037a9b2ac..1387779a85 100644 --- a/fs/squashfs/sqfs_inode.c +++ b/fs/squashfs/sqfs_inode.c @@ -138,11 +138,14 @@ void *sqfs_find_inode(void *inode_table, int inode_number, __le32 inode_count, int sqfs_read_metablock(unsigned char *file_mapping, int offset, bool *compressed, u32 *data_size) { - unsigned char *data; + const unsigned char *data; u16 header; data = file_mapping + offset; header = get_unaligned((u16 *)data); + if (!header || !data) + return -EINVAL; + *compressed = SQFS_COMPRESSED_METADATA(header); *data_size = SQFS_METADATA_SIZE(header);