From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1lMp6M-0000RY-8i for mharc-grub-devel@gnu.org; Thu, 18 Mar 2021 05:37:58 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:40482) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lMp6L-0000OL-C9 for grub-devel@gnu.org; Thu, 18 Mar 2021 05:37:57 -0400 Received: from aserp2130.oracle.com ([141.146.126.79]:44190) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lMp6I-000763-BH for grub-devel@gnu.org; Thu, 18 Mar 2021 05:37:56 -0400 Received: from pps.filterd (aserp2130.oracle.com [127.0.0.1]) by aserp2130.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 12I9OJOq094143; Thu, 18 Mar 2021 09:37:44 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=from : to : cc : subject : in-reply-to : references : date : message-id : content-type : mime-version; s=corp-2020-01-29; bh=aP9wg+IWFQ9o9jf0ib4VDrvBdKPC1tRnQM++vFJDMAQ=; b=cHx7X2LysJOLNVk6MLqTs1+X8qGYXzitghkes2X3tPR7/q59Kgf3ESaWRNY6LMWYxcDg iM+aYcSNyZaqMUQNn5meAZvqQAIpHBOgAnwmbHxys/BMhEGbTmlRH9EHmCwZn+TyiHwu nZXpFwbcmmCOFo+QpVPzKPea+IOzVOfcu8oM/wJ5h1bjRWZH45ASlrEXsQrbk+y5DHwK pIY6b3BL0i/Vhny1ZHyXFFwUkoilUnR3Xrh2p0SYxe6MGEB1DYnazbsiD03ICJDsjVH7 a12wK0lNxNUJ/xs4WrcXSERInaR2EWUaxKfEje76NdP/JdSY8VbeamAz6Y+P1BXGCjHY Yw== Received: from userp3030.oracle.com (userp3030.oracle.com [156.151.31.80]) by aserp2130.oracle.com with ESMTP id 378jwbpxrd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 18 Mar 2021 09:37:44 +0000 Received: from pps.filterd (userp3030.oracle.com [127.0.0.1]) by userp3030.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 12I9Q5de022728; Thu, 18 Mar 2021 09:37:43 GMT Received: from nam12-dm6-obe.outbound.protection.outlook.com (mail-dm6nam12lp2174.outbound.protection.outlook.com [104.47.59.174]) by userp3030.oracle.com with ESMTP id 3797b2pqrx-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 18 Mar 2021 09:37:43 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=jUQM018JZGud73K+hjnV7L7WsVKn5x9AE4LhkYsHzB+1egbyln6JsmkeDCA1ksXDa20xdXtC1bDfL2F+qhEI3mIZNNW81BTzHq11cRlIgW/41/e5Ih+Wh7yzcDBVbDjqRJOn404XRaGPixIcsPJsRhVefPo7n3d1gaDNI/Y98zzum1yFdK/82opJmW3c9XxtTxDKogjevTbvv/iplUr2RQN3w9WnG1GwaOxtkmqXVS+wG3VtxdRY7uYwkfstw5YHjOwTWY1SCD18yI/JkUgxq4D5IKfllmwaISxMOaucg9bcphZtrkAKnrd6Xn1GYiJzdrjxD5nYK5uwFETv2UptmA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=aP9wg+IWFQ9o9jf0ib4VDrvBdKPC1tRnQM++vFJDMAQ=; b=TUUpKNEENPa/b6wTwK01mOefzr1Fgyd+VF8SOFgSqay8BNiEGqUIHp1pRTIO0f80zH6iyzd9xTTclhg+Qi5E84QTm6Q0+be/bl1Z9pprEzu8bu+wmvPofP4Bvb/KJYsaLgVk9IKfjf9gGOLpRkkKyvkX4sTbxaJFYYo1RNtdhNsKePZYvAovYMNIIszQtNk12wM1z168a5YMA45/cuBxkeTKl8MNhymeJbKg+jTGqy5jBNMvd5tasgDEvEvPwb0XVb2KdrPH0tUDAPD9Z0uqlsSaMP2XBExdxsq4e+qZgz0qmwygbReFWQx0NkKMj1n98S5zUrlOtnklUmCKwD5b4Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.onmicrosoft.com; s=selector2-oracle-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=aP9wg+IWFQ9o9jf0ib4VDrvBdKPC1tRnQM++vFJDMAQ=; b=VkUqFNG3sZnpnKBmvkWPHQWnSvsii9sFHZ3pRflrbpPxEPpxfchrA9+tXOCw41Zu0aOweLa4d/D5p3CXiaRzp2ZOsJrfbd7lz1U69wG7lwX5rk1Ouz+c0HVyz8qRs/+YO+cSRWXJ4Ya6I+H/jC9tIRsrbeUnfOOlwXOjziNpwWg= Authentication-Results: molgen.mpg.de; dkim=none (message not signed) header.d=none;molgen.mpg.de; dmarc=none action=none header.from=oracle.com; Received: from DM6PR10MB2857.namprd10.prod.outlook.com (2603:10b6:5:64::25) by DM6PR10MB2809.namprd10.prod.outlook.com (2603:10b6:5:63::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3933.32; Thu, 18 Mar 2021 09:37:41 +0000 Received: from DM6PR10MB2857.namprd10.prod.outlook.com ([fe80::fca8:448b:525f:7873]) by DM6PR10MB2857.namprd10.prod.outlook.com ([fe80::fca8:448b:525f:7873%7]) with mapi id 15.20.3933.032; Thu, 18 Mar 2021 09:37:41 +0000 From: Darren Kenny To: Paul Menzel , Daniel Kiper Cc: Nick Terrell , The development of GNU GRUB Subject: Re: [SECURITY PATCH 029/117] zstd: Initialize seq_t structure fully In-Reply-To: <2abf6990-ef2a-5563-fa97-6ea2f25d5bfa@molgen.mpg.de> References: <20210302180056.zq4bk2w2cuqhbvx3@tomti.i.net-space.pl> <20210302180204.23887-29-daniel.kiper@oracle.com> <2abf6990-ef2a-5563-fa97-6ea2f25d5bfa@molgen.mpg.de> Date: Thu, 18 Mar 2021 09:37:36 +0000 Message-ID: Content-Type: text/plain X-Originating-IP: [79.97.215.145] X-ClientProxiedBy: DB6PR0301CA0084.eurprd03.prod.outlook.com (2603:10a6:6:30::31) To DM6PR10MB2857.namprd10.prod.outlook.com (2603:10b6:5:64::25) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from oracle.com (79.97.215.145) by DB6PR0301CA0084.eurprd03.prod.outlook.com (2603:10a6:6:30::31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3955.18 via Frontend Transport; Thu, 18 Mar 2021 09:37:41 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: dc483ed3-975d-45c2-86c0-08d8e9f179c9 X-MS-TrafficTypeDiagnostic: DM6PR10MB2809: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Cy+E5R+soCwBsTXd8U0ZjACi+px4HUlGRkz8NKmURZ6tfFJme8GGxixoeTk6f4+rzF/+R8QALwLgyhwqu49SoPkNlC8/xxZJEBR7iDN5uN32Q+z6Hqc6/q8NOYwJNndJkG2dxbsQXb/b2uolNstKsvhxHWUT6vjNWkEdX+S7wqJ4B2S+Xu0QvBk8xfsdptIs8aAseu4e6bEfTFx5115s9Bavuarv3PTK5e/bt9hHRicKr32/MH5lydH5/6q54ONBetmRiCqFD6i3mAsrUAr+eShZ27tBGuAwhsj2wuveeJDOglEaC5bpp9jQ+TyBce+zOWQU17GT8xhh7aMeZrx2q5ppTSUTUtLigbz+s+bH2wE7qTsafes0UPQehL4f/GtnHw8Qsp/2m2pBsjdTgbyGy95rqlyGjQheWnJlKNA069hFohzhMFgu2oJKkBIEYydSDuQ4e1f6hzxFmqE1/aOL61ktVOvPoCeTHwehdqAK+TExdYsag5ArgrvEAzuo+Mbsox1uwjPAzBod2Y55Pxj6kAkaOvxNRQhx2LDwNejSRkR7AGd9nSXJ9eHavPHbQkhFjMpVY8HIFGkp4WzwcCroZCJDY926fkQQfjftA9dV9MLtf5IVicjhQSKFn1RLYuQ2kihIo8VZdRbGEwP1hXZcEdqkB1LSN0rKZ46EzQclOvWb7FFiS69RdPV8aFymy+u5dzoa97GUEgESp5aQKUqZHQ== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR10MB2857.namprd10.prod.outlook.com; PTR:; CAT:NONE; SFS:(396003)(39860400002)(346002)(136003)(376002)(366004)(83380400001)(6666004)(6636002)(316002)(19627235002)(66476007)(38100700001)(8676002)(110136005)(4326008)(52116002)(44832011)(8886007)(5660300002)(956004)(54906003)(26005)(8936002)(16526019)(66946007)(55016002)(186003)(86362001)(15650500001)(478600001)(2906002)(966005)(2616005)(36756003)(66556008)(7696005); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?us-ascii?Q?tezJhHa6GDLmnAwRn0lTW3TnsQQ7OpYtENNEwQrPejC9LjMTOmgMB4oVxAyJ?= =?us-ascii?Q?41uPEb01RbdLgVRkxcdDZGcoxtZmIZ+kc0CDBgwg8jq/t+n22N3XnmgACZlG?= =?us-ascii?Q?qKHWTWHGP+WZt8hHA5YG/04OoKIhBaUl0DF94ZBPHeENKHCZxp92Ci2Z4/9g?= =?us-ascii?Q?x2N2BjBXFvlv0crftogCOctRUx5W7J7yAlFDFOLWH90lrUg1jFZ8e9lwn5W8?= =?us-ascii?Q?eiN8sJasxwkRYcCLYP7RRcSGApAsSCeADcYu5EMptILk5k2WoalsbWrUzOWs?= =?us-ascii?Q?vD2GnmNzL3e+u9j99LfYrwZeM3V2ows9BiXyCdPG5mUsDwDc94Oc8pDpsTp4?= =?us-ascii?Q?qdDXhBjqKRDmFfa62pFfVhwdYz3BC3AtGoPGOzVkYbJ7CfcvCPRNuwM3tq6i?= =?us-ascii?Q?XmBMzXjZKElm48Q+XUcI/B6rrfkl6qmuAPZPnUZSgF23q9YMrMVxvCHbAsEB?= =?us-ascii?Q?Mym9Xgme7iVd++Se64o8AuSRHFPqZPmWUVIFpMKFvLtzbb9g0uooSyMEBSfa?= =?us-ascii?Q?Ovwq5wivSbirBjZugS25/gzn4NVD9ylCSpfBQxx72dn30kG5Q/H3QAKLsq5f?= =?us-ascii?Q?AcWm3nwx6/yKuZGCGaOa7NvdljVg8NsmIga6HSWsuxgLyS3X5vP8meG19M8M?= =?us-ascii?Q?5MbMWhBo4Nj1OccFMvczlalo742jy23VVxFda12HRoQArDrJymAXYlS/Foyj?= =?us-ascii?Q?CX7vqGlVx7tbE9ZzNmPZrnKZ+En995Dl0RzewjO1M3xvPSbOJfnQpKI2RKL3?= =?us-ascii?Q?EonTTg+CW9oc9L3Zon0g1MENgILqcvZ0jVxngbsmGVNPvrJWUBIsq5EHT756?= =?us-ascii?Q?ZsYSkzNMMDKqCufL2o3OG+rTY6DGJujBcwgDDdHjBiB+o0Pa0ggFAXT7yUWh?= =?us-ascii?Q?IvBVh//MH+P+7IcsOGVp6+uswvZD1wJ5yAX34OCRhd24rweXDaFwoqrtPk77?= =?us-ascii?Q?Tsha6uh7ABHoUPEkY88f6+6zecDEKzLGDETWjO9BV5oeDEx+2DeB6xq2+iHz?= =?us-ascii?Q?qttdmaaYufgjBk9CoUCuLD7DmLZShwfP2TkgIRfRNskNDVMp4FRU53J5XMYF?= =?us-ascii?Q?qlDq67L/+rTBRN295bXdJTFOZdEZW3rmME8UtncrOgOFpOEEmwPBZ2pAg3Hy?= =?us-ascii?Q?BwyOCpvbTfDpPa7NltZHwh9W7N8c5dfM7yr0TD5gfaJUQqCuo6+C8Q2MR6dl?= =?us-ascii?Q?jYgjwJ1dCrEsWOatCwzsOZr21faliqDu1YzicCqMB0LIuJNCH4LZw7ljvfVQ?= =?us-ascii?Q?9wh5Jcd0WLJ/L8FU/n8qXTBYv4H7OGYkWPwbrBNzYNsB/iYeItvedAD/fpeO?= =?us-ascii?Q?tefq+tTloS2fJUYCC/JXunaM?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: dc483ed3-975d-45c2-86c0-08d8e9f179c9 X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB2857.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Mar 2021 09:37:41.3854 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 6G/0+m2ItmtE79r9ifMAkg/tIVzZQn9M4v9i2rtGogzZkAmuEOFcmk5q1J7ew/ScpMWk05WxkLICexoqqbbcNw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB2809 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9926 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 spamscore=0 bulkscore=0 malwarescore=0 adultscore=0 mlxscore=0 mlxlogscore=999 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2103180071 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9926 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxlogscore=999 spamscore=0 mlxscore=0 bulkscore=0 suspectscore=0 priorityscore=1501 lowpriorityscore=0 clxscore=1011 adultscore=0 phishscore=0 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2103180071 Received-SPF: pass client-ip=141.146.126.79; envelope-from=darren.kenny@oracle.com; helo=aserp2130.oracle.com X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, MSGID_FROM_MTA_HEADER=0.001, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Mar 2021 09:37:57 -0000 Hi Paul, On Thursday, 2021-03-18 at 09:50:00 +01, Paul Menzel wrote: > Dear Darren, dear Daniel, > > > Am 02.03.21 um 19:00 schrieb Daniel Kiper: >> From: Darren Kenny >> >> While many compilers will initialize this to zero, not all will, > > Which ones do not? I have been working with C for a long time now, and there have been compilers that don't initialize stack variables. While many of the Linux compilers such as gcc and clang compilers often do - depending on the optimization levels and building with debug or not. (Debug code tends to be correctly zeroed out, but optimized doesn't always) I remember having to change a lot of GNOME C code being ported to another platform, Solaris at the time, where that code always assumed that it would be initialized to 0, and random things would happen depending on what was on the stack. The details of this are well documented and are a known security issue: - CWE-457 (http://cwe.mitre.org/data/definitions/457.html) >> so it is better to be sure that fields not being explicitly set are at known >> values, and there is code that checks this fields value elsewhere in the >> code. >> >> Fixes: CID 292440 > > What is the exact error? Is there a code flow, where one element does > not get set. (The commit message would be incorrect if this is not the > case.) I can't honestly remember the details that Coverity had, but there was a flow found by Coverity that did end up returning the value in seq without touching all of the fields in the structure. Looking through the code manually just now, I can see that seq.match is never set during that code in ZSTD_decodeSequence(), so that is the most likely cause of the error from Coverity. All that it can do is report that it was not set before returning, it cannot necessarily predict what will happen after that function returns it, especially over time. > > Lastly, this is imported from upstream. I created an issue upstream [1]. > Thanks, makes sense. >> Signed-off-by: Darren Kenny >> Reviewed-by: Daniel Kiper >> --- >> grub-core/lib/zstd/zstd_decompress.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/grub-core/lib/zstd/zstd_decompress.c b/grub-core/lib/zstd/zstd_decompress.c >> index 711b5b6d7..e4b5670c2 100644 >> --- a/grub-core/lib/zstd/zstd_decompress.c >> +++ b/grub-core/lib/zstd/zstd_decompress.c >> @@ -1325,7 +1325,7 @@ typedef enum { ZSTD_lo_isRegularOffset, ZSTD_lo_isLongOffset=1 } ZSTD_longOffset >> FORCE_INLINE_TEMPLATE seq_t >> ZSTD_decodeSequence(seqState_t* seqState, const ZSTD_longOffset_e longOffsets) >> { >> - seq_t seq; >> + seq_t seq = {0}; >> U32 const llBits = seqState->stateLL.table[seqState->stateLL.state].nbAdditionalBits; >> U32 const mlBits = seqState->stateML.table[seqState->stateML.state].nbAdditionalBits; >> U32 const ofBits = seqState->stateOffb.table[seqState->stateOffb.state].nbAdditionalBits; >> > > I once read, that compilers cannot warn you, if you miss setting an > element if you initialize structures to 0 in the beginning. > That is true, since it has been initialized :) But compilers won't always warn you either unless you ask them to, e.g. gcc requires you to add the -Wuninitialized option, it isn't included in -Wall (at lot aren't TBH, despite the name). It is fine not to initialize while developing the code - if you want to use that to catch such cases - but you should probably then also be enabling every possible warning that a compiler may provide, maybe also using some static code analysis (though compilers are improving a lot here, e.g. in GCC 10) and most importantly fix them all before shipping code :) All to common is that people end up turning off the annoying warnings rather than paying attention to them and resolving them. Thanks, Darren. > Kind regards, > > Paul > > > [1]: https://github.com/facebook/zstd/issues/2545