From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 055FEC4338F for ; Mon, 16 Aug 2021 20:20:41 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 485E160F22 for ; Mon, 16 Aug 2021 20:20:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 485E160F22 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id E471180EC5; Mon, 16 Aug 2021 22:20:37 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="Gku/gyC3"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id BF5CD80EC5; Mon, 16 Aug 2021 22:20:35 +0200 (CEST) Received: from mail-qk1-x735.google.com (mail-qk1-x735.google.com [IPv6:2607:f8b0:4864:20::735]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 36FD48060B for ; Mon, 16 Aug 2021 22:20:31 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qk1-x735.google.com with SMTP id 14so20500135qkc.4 for ; Mon, 16 Aug 2021 13:20:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=+CVDaIVBk9oSkOfu/8gLj8C3z3IZJzIGpMO1MCEGTTc=; b=Gku/gyC3utPG5gGHNdyayfcB08EZEa2FOw3BM+nIPEtR+lETkwjLDKVdFoXaEylavE DO4ed0fOEQ9tlhnfnlMI7jR6e7PzfduwdTmKLHvrj5RNpoxaRdMKj4KRDrYwYcbB8h+N 246JF5/KtxET0nqFlMROeUGI/BRbXjVV8L0fI= 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:references :mime-version:content-disposition:in-reply-to:user-agent; bh=+CVDaIVBk9oSkOfu/8gLj8C3z3IZJzIGpMO1MCEGTTc=; b=da3SATgatZWsuUtEP41K2xxRCe+1l74p2WOI1+Um0i2oP+cTHbezA2NxtbKhuhZVvM Qmy+1u3ObpZUu6WN6iqyvhEZaIchnzK4SEJ2CXv2l0IIER0tWcKAaXoO8mtW+qeUohKL 3dnDe4aSEQB76aJy+zYQH02jE5JrwhDr06yo5jVTOW8PzgWXI4xEC/77XfkaozHeAtMC AwxJbFBvk95zvV8/RtxTldkzhODnEg4MNQsOmoFetCrH7RWLEw8S5mmX8zX1VWy/2s2a 6vu0USImKgL6SOqo79jWAkh1TpfYNOJPPb3xi2ua5K4iY2pJtgZ/Opne+bT17vq+GG9D +9iA== X-Gm-Message-State: AOAM5322EhnETapiX4GgswtEUWztEJnau4amUGggi6LWBoS6Hf2MmwoY JqXHsuBq5i+2kvRdsoHpFQqSpg== X-Google-Smtp-Source: ABdhPJyxc4AyWpYKoWpx41YH0vlrvjQ7DmF4QinDqOmw8Mc076Q/Q6IOTC5C41s54HeC8qYxaoJbAg== X-Received: by 2002:a05:620a:113b:: with SMTP id p27mr104164qkk.143.1629145230002; Mon, 16 Aug 2021 13:20:30 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b01-cbda-052e-d62b-d66a-f817.res6.spectrum.com. [2603:6081:7b01:cbda:52e:d62b:d66a:f817]) by smtp.gmail.com with ESMTPSA id l4sm156230qkd.77.2021.08.16.13.20.29 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 16 Aug 2021 13:20:29 -0700 (PDT) Date: Mon, 16 Aug 2021 16:20:27 -0400 From: Tom Rini To: Pali =?iso-8859-1?Q?Roh=E1r?= Cc: u-boot@lists.denx.de, Simon Glass , Alexandru Gagniuc , Stefan Roese , Marek =?iso-8859-1?Q?Beh=FAn?= Subject: Re: [scan-admin@coverity.com: New Defects reported by Coverity Scan for Das U-Boot] Message-ID: <20210816202027.GE858@bill-the-cat> References: <20210816195726.GD858@bill-the-cat> <20210816201549.kzjonaqei2rwpefq@pali> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="bTTI3rp5igVUz0aj" Content-Disposition: inline In-Reply-To: <20210816201549.kzjonaqei2rwpefq@pali> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.9.4 (2018-02-28) 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.103.2 at phobos.denx.de X-Virus-Status: Clean --bTTI3rp5igVUz0aj Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Aug 16, 2021 at 10:15:49PM +0200, Pali Roh=E1r wrote: > + Stefan and Marek >=20 > On Monday 16 August 2021 15:57:26 Tom Rini wrote: > > Hey all, > >=20 > > Can people please take a look? I can mark as intentional anything that > > really is intentional, thanks. >=20 > Hello Tom! >=20 > These kwbimage issues look to be a real issues. But I do not think that > anybody touched these parts of kwbimage code recently. So looks like > that Coverity must have run some more tests this time... Yeah, that happens from time to time. >=20 > > ** CID 338491: Null pointer dereferences (NULL_RETURNS) > > /tools/kwbimage.c: 1066 in export_pub_kak_hash() > >=20 > >=20 > > _______________________________________________________________________= _________________________________ > > *** CID 338491: Null pointer dereferences (NULL_RETURNS) > > /tools/kwbimage.c: 1066 in export_pub_kak_hash() > > 1060 int res; > > 1061 =20 > > 1062 hashf =3D fopen("pub_kak_hash.txt", "w"); > > 1063 =20 > > 1064 res =3D kwb_export_pubkey(kak, &secure_hdr->kak, hashf, "KAK"= ); > > 1065 =20 > > >>> CID 338491: Null pointer dereferences (NULL_RETURNS) > > >>> Dereferencing a pointer that might be "NULL" "hashf" when calli= ng "fclose". > > 1066 fclose(hashf); > > 1067 =20 > > 1068 return res < 0 ? 1 : 0; > > 1069 } > > 1070 =20 > > 1071 int kwb_sign_csk_with_kak(struct image_tool_params *params, >=20 > There is really missing check that fopen() succeeded. >=20 > > ** CID 338488: Memory - illegal accesses (NEGATIVE_RETURNS) > > /tools/kwbimage.c: 1093 in kwb_sign_csk_with_kak() > >=20 > >=20 > > _______________________________________________________________________= _________________________________ > > *** CID 338488: Memory - illegal accesses (NEGATIVE_RETURNS) > > /tools/kwbimage.c: 1093 in kwb_sign_csk_with_kak() > > 1087 if (export_pub_kak_hash(kak, secure_hdr)) > > 1088 return 1; > > 1089 =20 > > 1090 if (kwb_import_pubkey(&kak_pub, &secure_hdr->kak, "KAK") < 0) > > 1091 return 1; > > 1092 =20 > > >>> CID 338488: Memory - illegal accesses (NEGATIVE_RETURNS) > > >>> Using variable "csk_idx" as an index to array "secure_hdr->csk". > > 1093 if (kwb_export_pubkey(csk, &secure_hdr->csk[csk_idx], NULL, "= CSK") < 0) > > 1094 return 1; > > 1095 =20 > > 1096 if (kwb_sign_and_verify(kak, &secure_hdr->csk, > > 1097 sizeof(secure_hdr->csk) + > > 1098 sizeof(secure_hdr->csksig), >=20 > There is code: >=20 > int csk_idx =3D image_get_csk_index(); > ... > if (csk_idx >=3D 16) { > ... > return 1; > } > ... &secure_hdr->csk[csk_idx] ... >=20 > And ->csk is defined as: >=20 > struct secure_hdr_v1 { > .. > struct pubkey_der_v1 csk[16] > .. > }; >=20 > image_get_csk_index() returns int and it may returns also negative value > on error. So there is really possible illegal memory access. >=20 > > ** CID 338486: Null pointer dereferences (NULL_RETURNS) > > /tools/kwbimage.c: 836 in kwb_dump_fuse_cmds() > >=20 > >=20 > > _______________________________________________________________________= _________________________________ > > *** CID 338486: Null pointer dereferences (NULL_RETURNS) > > /tools/kwbimage.c: 836 in kwb_dump_fuse_cmds() > > 830 return 0; > > 831 =20 > > 832 if (!strcmp(e->name, "a38x")) { > > 833 FILE *out =3D fopen("kwb_fuses_a38x.txt", "w+"); > > 834 =20 > > 835 kwb_dump_fuse_cmds_38x(out, sec_hdr); > > >>> CID 338486: Null pointer dereferences (NULL_RETURNS) > > >>> Dereferencing a pointer that might be "NULL" "out" when calling= "fclose". > > 836 fclose(out); > > 837 goto done; > > 838 } > > 839 =20 > > 840 ret =3D -ENOSYS; > > 841 =20 >=20 > And there is also missing check that fopen() succeeded. Since you've been in here and analyzed things (thanks!) can you make a few patches for things? --=20 Tom --bTTI3rp5igVUz0aj Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmEayIcACgkQFHw5/5Y0 tyx1Ewv/bpzlIrMR6Kv2BDg4jbeOe2rzS3fQ2iqyGYYCwvfdywv/TapuMGvaQxfn vsrA+E8MeggedMyzofw7LvXXFXOL5MpUbQcttNqg9LKQVbieB1lxoOTONUC39K1G UxmYcaBKtNTjJw9hwmS0QnRS4XJuAl4BbBX/9HH6600Rjq9NXLsZPWsciIvNJ73a rBhlYVZh6w4B9bpFT5eTzMPRQxP0gNXBb1ft9epsebM0hyNiBi/739NS434QNICq +RFa7SvTLPtNyj1G/DyJ+xgh0DV9WjNxacUSj4dxtOn8w2fHopNiHKtG8aqIrJzj AolaUpB5XxlXgIotXsiH51Rxf5sKBYlRpWfob2+8aZJOO6kXhXkH5vRQ6ok6mpZT bfgv/1BD1bVmWzFpd9wpJhhYMW+hm+nz/PFrJd9rG8pbW2IJZwMMqn3fk4DUiusS wXqYuXAEtGRMaABsmAQazE5/sjxBGDS0A9EHxF37LTT++z9XHlH7Cr+rWqxKRWzK N6fAimNk =eLlY -----END PGP SIGNATURE----- --bTTI3rp5igVUz0aj--