From patchwork Fri Sep 14 20:47:49 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Warren X-Patchwork-Id: 970101 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-tegra-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=wwwdotorg.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 42BnrV29z8z9s2P for ; Sat, 15 Sep 2018 06:56:57 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727773AbeIOCNF (ORCPT ); Fri, 14 Sep 2018 22:13:05 -0400 Received: from avon.wwwdotorg.org ([104.237.132.123]:51354 "EHLO avon.wwwdotorg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727416AbeIOCNF (ORCPT ); Fri, 14 Sep 2018 22:13:05 -0400 X-Greylist: delayed 523 seconds by postgrey-1.27 at vger.kernel.org; Fri, 14 Sep 2018 22:13:05 EDT Received: from swarren-lx1.nvidia.com (unknown [216.228.112.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by avon.wwwdotorg.org (Postfix) with ESMTPSA id 330121C01A2; Fri, 14 Sep 2018 14:48:15 -0600 (MDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.100.1 at avon.wwwdotorg.org From: Stephen Warren To: linux-tegra@vger.kernel.org Cc: Stephen Warren Subject: [cbootimage PATCH] Fix various abort(), crashes, and memory errors Date: Fri, 14 Sep 2018 14:47:49 -0600 Message-Id: <20180914204749.11014-1-swarren@wwwdotorg.org> X-Mailer: git-send-email 2.18.0 X-NVConfidentiality: public Sender: linux-tegra-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-tegra@vger.kernel.org From: Stephen Warren cbootimage doesn't have extensive error-checking of the input files. Thus it's easy to trigger aborts (which in turn segfault to exit the app) and bad memory accesses by providing under-sized binary input files or configuration files with missing required statements. Add a bit more error-checking to clean up some of these cases. No doubt there are more, but this change only fixes those that have been reported. Signed-off-by: Stephen Warren Reviewed-by: Thierry Reding --- src/cbootimage.c | 15 ++++++++++++++- src/data_layout.c | 11 +++++++++-- src/parse.c | 2 +- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/cbootimage.c b/src/cbootimage.c index ca781fa6dab1..e728c8b06801 100644 --- a/src/cbootimage.c +++ b/src/cbootimage.c @@ -239,7 +239,7 @@ main(int argc, char *argv[]) /* Get BCT_SIZE from input image file */ bct_size = get_bct_size_from_image(&context); - if (bct_size < 0) { + if (bct_size <= 0) { printf("Error: Invalid input image file %s\n", context.input_image_filename); goto fail; @@ -301,6 +301,19 @@ main(int argc, char *argv[]) goto fail; } + if (!context.bct_init) { + e = 1; + printf("No BCT file has been read or generated.\n"); + printf("This is likely due to an incomplete config file.\n"); + goto fail; + } + if (!context.memory) { + e = 1; + printf("No output data generated.\n"); + printf("This is likely due to an incomplete config file.\n"); + goto fail; + } + /* Peform final signing & encryption of bct. */ e = sign_bct(&context, context.bct); if (e != 0) { diff --git a/src/data_layout.c b/src/data_layout.c index 2ed486bf12b5..8301aec59b03 100644 --- a/src/data_layout.c +++ b/src/data_layout.c @@ -218,8 +218,10 @@ write_page(build_image_context *context, return -ENOMEM; if (block->data == NULL) return -ENOMEM; - assert(((page_number + 1) * context->page_size) - <= context->block_size); + if (((page_number + 1) * context->page_size) > context->block_size) { + printf("Page number outside block; likely config file error.\n"); + return -ENOMEM; + } if (block->pages_used != page_number) { printf("Warning: Writing page in block out of order.\n"); @@ -838,6 +840,11 @@ begin_update(build_image_context *context) assert(context); + if (context->page_size_log2 < NVBOOT_AES_BLOCK_SIZE_LOG2) { + printf("Page size is too small; likely config file error\n"); + return 1; + } + /* Ensure that the BCT block & page data is current. */ if (enable_debug) { uint32_t block_size_log2; diff --git a/src/parse.c b/src/parse.c index 99cb428b26fd..10060936c529 100644 --- a/src/parse.c +++ b/src/parse.c @@ -249,7 +249,7 @@ parse_filename(char *str, char *name, int chars_remaining) * Check if the filename buffer is out of space, preserving one * character to null terminate the string. */ - while (isalnum(*str) || strchr("\\/~_-+:.@", *str)) { + while (*str && (isalnum(*str) || strchr("\\/~_-+:.@", *str))) { chars_remaining--;