From patchwork Sat Nov 24 02:18:37 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Teddy Reed X-Patchwork-Id: 1002552 X-Patchwork-Delegate: sjg@chromium.org 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=lists.denx.de (client-ip=81.169.180.215; helo=lists.denx.de; envelope-from=u-boot-bounces@lists.denx.de; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="MHeG33XI"; dkim-atps=neutral Received: from lists.denx.de (dione.denx.de [81.169.180.215]) by ozlabs.org (Postfix) with ESMTP id 431xgZ3hHSz9s3C for ; Sat, 24 Nov 2018 13:18:49 +1100 (AEDT) Received: by lists.denx.de (Postfix, from userid 105) id 34178C2245D; Sat, 24 Nov 2018 02:18:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de X-Spam-Level: X-Spam-Status: No, score=0.0 required=5.0 tests=FREEMAIL_FROM, RCVD_IN_MSPIKE_H2, T_DKIM_INVALID autolearn=unavailable autolearn_force=no version=3.4.0 Received: from lists.denx.de (localhost [IPv6:::1]) by lists.denx.de (Postfix) with ESMTP id 9540FC22442; Sat, 24 Nov 2018 02:18:42 +0000 (UTC) Received: by lists.denx.de (Postfix, from userid 105) id 93706C22442; Sat, 24 Nov 2018 02:18:41 +0000 (UTC) Received: from mail-qk1-f194.google.com (mail-qk1-f194.google.com [209.85.222.194]) by lists.denx.de (Postfix) with ESMTPS id 2EB6CC2243B for ; Sat, 24 Nov 2018 02:18:40 +0000 (UTC) Received: by mail-qk1-f194.google.com with SMTP id 131so9408840qkd.4 for ; Fri, 23 Nov 2018 18:18:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:mime-version :content-transfer-encoding; bh=SJJmzCpElTfTPZIfgXu1TAlYyEj8ZcxFrHM+kIQ9VBY=; b=MHeG33XIGAq9te40dAvW1xxfHDWcUO961yp7o9XnUp1ZvtygZxayfaUo+/MFlxYgxx 7U8qi0mqnJclZcIJuTarwAlW0P7FewKbSR8sUKQ799PAn2k8E9QLGAzoheeMa8Zb3irS ZqkW4T3YNgc5z80u7WBHNvxsjL20GQ/Wn7Si+fBgIvJcXXTru9PRkfCbODkJGrf3DhV5 pg2ZNQV1a4UUM7uMzo3a/eOEL1WMqTYuGrn/VI3KvRth+lQVfe8DqpOWZzKM5rQ0jGZJ meEpJfiIEE4qYKJCB/B4oHByslOf+PpWZjvYjl3+B+rNKlavRJ8llR5VJMmadTPu4jm/ kjaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-transfer-encoding; bh=SJJmzCpElTfTPZIfgXu1TAlYyEj8ZcxFrHM+kIQ9VBY=; b=brPKZQ1yZ2QZr4WNc2kG++HZfPmJ/rF83+/tHl/8iStbBbGfQn0YcHhJpZC6cpbBDb bDrGpJt+Ukwv/lWErfvh+qkGpwk+j3TeaO4geylDyaDN/2E+Y/fF6Ysu28xVVccr62z9 d2L76UJpwk5St6pltafIaWgjmnl+CHaiZ/wFAt6pCwgjZlLcv/wynaHph7LoBrWynvsO d7g12yho4pbvQ20uvOkYQbNYuMOXGjcmFeRNtMbJJS1RthkmCCLGLFtsWaiDWpmOJUqu 7GJOtTcGTDgVG8+9pLCLJoxN8Zgk6qR4EsxEf+4l2pNcNNqhHD+A6fIM/njqo5UmJ2fq Ae/g== X-Gm-Message-State: AA+aEWZp4AtkRQ9YYkHADFnbnL4R1523RwujXV8bHSzsoalrxgMn2Pup 91SjYi0//D5jsNmTO83uNwc9n3MZRIA= X-Google-Smtp-Source: AFSGD/Wr4IbPFhUSgaZN8tm65FUxyX9FXtiW3hU86hYiVhoekHxw4Z/EuuSSzRBgSHFjqZ0xicRRjw== X-Received: by 2002:a37:1745:: with SMTP id i66mr16211512qkh.125.1543025918699; Fri, 23 Nov 2018 18:18:38 -0800 (PST) Received: from food (pool-173-70-222-41.nwrknj.fios.verizon.net. [173.70.222.41]) by smtp.gmail.com with ESMTPSA id f31sm23032611qtc.42.2018.11.23.18.18.37 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 23 Nov 2018 18:18:37 -0800 (PST) Date: Fri, 23 Nov 2018 21:18:37 -0500 From: Teddy Reed To: u-boot@lists.denx.de Message-Id: <20181123211837.274e66d349a55958981207e7@gmail.com> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.32; x86_64-pc-linux-gnu) Mime-Version: 1.0 Cc: david@gibson.dropbear.id.au Subject: [U-Boot] [PATCH] fdt: Fix string property comparison overflow X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.18 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" FDT property searching can overflow when comparing strings. This will result in undefined behavior. This upstream patch adds checks to assure property name lengths do not overrun the string region or the totalsize. The implementation is from upstream dtc: 70166d62a27f libfdt: Safer access to strings section Cc: Peter Robinson Cc: David Gibson Signed-off-by: Teddy Reed --- Note this file is not synchronized from upstream dtc when using the scripts/dtc/update-dtc-source.sh script. The file size of the ELF increases with sandbox_spl_defconfig. $ bloaty spl/u-boot-spl -- spl/u-boot-spl.bin.before VM SIZE FILE SIZE -------------- -------------- [ = ] 0 .debug_loc +948 +0.3% [ = ] 0 .debug_info +284 +0.0% +0.5% +176 .text +176 +0.5% [ = ] 0 .debug_line +150 +0.2% [ = ] 0 .debug_ranges +48 +0.2% +0.4% +40 .eh_frame +40 +0.4% [ = ] 0 .debug_str +20 +0.0% [ = ] 0 .debug_aranges +16 +0.1% +59% +16 [LOAD [RX]] +16 +59% [ = ] 0 .strtab +4 +0.0% [ = ] 0 [Unmapped] -238 -18.1% +0.3% +232 TOTAL +1.43Ki +0.1% $ du -b spl/u-boot-spl.bin spl/u-boot-spl.bin.before 2174032 spl/u-boot-spl.bin 2174032 spl/u-boot-spl.bin.before You could also apply the patch: @@ -42,6 +42,11 @@ const char *fdt_string(const void *fdt, int stroffset) static int _fdt_string_eq(const void *fdt, int stroffset, const char *s, int len) { + int total_off = fdt_off_dt_strings(fdt) + stroffset; + if (total_off + len + 1 < total_off || + total_off + len + 1 > fdt_totalsize(fdt)) + return 0; + const char *p = fdt_string(fdt, stroffset); return (strnlen(p, len + 1) == len) && (memcmp(p, s, len) == 0); To mitigate the read overflow, with minimum added bytes. I proposed this along with another change [1]. This was not a good idea since the change to scripts/dtc/libfdt/fdt.c is part of dtc synchronized from upsteam. [1] https://lists.denx.de/pipermail/u-boot/2018-June/330488.html lib/libfdt/fdt_ro.c | 61 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 3 deletions(-) diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c index b6ca4e0b0c..f67dcb7fc9 100644 --- a/lib/libfdt/fdt_ro.c +++ b/lib/libfdt/fdt_ro.c @@ -34,17 +34,72 @@ static int _fdt_nodename_eq(const void *fdt, int offset, return 0; } +const char *fdt_get_string(const void *fdt, int stroffset, int *lenp) +{ + uint32_t absoffset = stroffset + fdt_off_dt_strings(fdt); + size_t len; + int err; + const char *s, *n; + + err = fdt_check_header(fdt); + if (err != 0) + goto fail; + + err = -FDT_ERR_BADOFFSET; + if (absoffset >= fdt_totalsize(fdt)) + goto fail; + len = fdt_totalsize(fdt) - absoffset; + + if (fdt_magic(fdt) == FDT_MAGIC) { + if (stroffset < 0) + goto fail; + if (fdt_version(fdt) >= 17) { + if (stroffset >= fdt_size_dt_strings(fdt)) + goto fail; + if ((fdt_size_dt_strings(fdt) - stroffset) < len) + len = fdt_size_dt_strings(fdt) - stroffset; + } + } else if (fdt_magic(fdt) == FDT_SW_MAGIC) { + if ((stroffset >= 0) + || (stroffset < -fdt_size_dt_strings(fdt))) + goto fail; + if ((-stroffset) < len) + len = -stroffset; + } else { + err = -FDT_ERR_INTERNAL; + goto fail; + } + + s = (const char *)fdt + absoffset; + n = memchr(s, '\0', len); + if (!n) { + /* missing terminating NULL */ + err = -FDT_ERR_TRUNCATED; + goto fail; + } + + if (lenp) + *lenp = n - s; + return s; + +fail: + if (lenp) + *lenp = err; + return NULL; +} + const char *fdt_string(const void *fdt, int stroffset) { - return (const char *)fdt + fdt_off_dt_strings(fdt) + stroffset; + return fdt_get_string(fdt, stroffset, NULL); } static int _fdt_string_eq(const void *fdt, int stroffset, const char *s, int len) { - const char *p = fdt_string(fdt, stroffset); + int slen; + const char *p = fdt_get_string(fdt, stroffset, &slen); - return (strnlen(p, len + 1) == len) && (memcmp(p, s, len) == 0); + return p && (slen == len) && (memcmp(p, s, len) == 0); } uint32_t fdt_get_max_phandle(const void *fdt)