From patchwork Mon Jun 14 15:51:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrzej Hunt X-Patchwork-Id: 12319275 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=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 6F9DCC2B9F4 for ; Mon, 14 Jun 2021 15:51:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3AD4761352 for ; Mon, 14 Jun 2021 15:51:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233671AbhFNPxY (ORCPT ); Mon, 14 Jun 2021 11:53:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46296 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233035AbhFNPxX (ORCPT ); Mon, 14 Jun 2021 11:53:23 -0400 Received: from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com [IPv6:2a00:1450:4864:20::42b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 213C6C061574 for ; Mon, 14 Jun 2021 08:51:20 -0700 (PDT) Received: by mail-wr1-x42b.google.com with SMTP id c5so15061773wrq.9 for ; Mon, 14 Jun 2021 08:51:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=1p2WaOD2ZwdKN14UkGwljtf/BsFG3qoPm4eI4vsEllA=; b=mWi4j73X/gkDpHQzwP+QuV3cVBD8MDgyjo/BGEBCH6cIj7wN5L9WCwnEPwHbOLPc7H CRxLPsHmjYel2voDl2c9xIH9P4O/9dR6L25kdjJAi4DVdn8UOdDfEwAYrgTDlTS01ZbT NYl2t2UPKcxPA5B3wRMt8YzjNRpbRqb1oug7mXb2pMkM9OkgswpSd5f+mXEZ6u8U++Ve ZS2idy/jzu7xtT4xyhwwWkbF85mtS1sy84UuwxFAHVnvM90LWLKVzrs0osRoq0rqUtvj 3EG4FNnujtKdbcaZP/W8J8R/lQs8vgUeHWsc2mP3L0Osrbq+EAM1q9iliUijbPhpg6+O yP2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=1p2WaOD2ZwdKN14UkGwljtf/BsFG3qoPm4eI4vsEllA=; b=LVEXxf5Qn+F3ZUNf8BzukkVx11BE62HCcndvieIesCFrDDnqxnvgLsbJOaxQSoyjZp xVUTMQbU02SD5ZNBDnwl67aAAf6/9FRVLMjTP0dNIEbwnOFkJr3p275NuVzevawE5ypR IZih6YuE5J2WBlV0wSrz+a21TJZWyDHKkG90wvWh+0B8Fno26VE/hm2MKnD783FY8GWL aSBLWdGrJUe2w/PAcEH0HpKA3Z6U9s6CfazWL8+ONkpelu/9BmKryJpsfJAOjn7mQAoC IaH0gTE+8OPGRfUUDz+k2fWVw6A3Bj0YHjpJX8KjPwhDbkIhF02zVSmaXMFLixtEAL3z gQSQ== X-Gm-Message-State: AOAM530TdSbMnur7LQl2XaKglQtFsJUhXgsrU5KEW3Yn0GlztMJ9gwYm KA8hYHCRmogJ4nFWeDhvShlnMx6IlSo= X-Google-Smtp-Source: ABdhPJzI1kbxBY4bQSyBpCjPADbw4BJVwMcUs1YuLPeslwlUT+D0am0hTGUYET5N04MJLj1tXQ2LCg== X-Received: by 2002:a5d:4f08:: with SMTP id c8mr19527783wru.197.1623685878790; Mon, 14 Jun 2021 08:51:18 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id p12sm18702756wme.43.2021.06.14.08.51.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Jun 2021 08:51:18 -0700 (PDT) Message-Id: <7659d4bf13c27ed0b1b793a19959b469063b85ec.1623685877.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Mon, 14 Jun 2021 15:51:14 +0000 Subject: [PATCH v2 1/3] bulk-checkin: make buffer reuse more obvious and safer Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Chris Torek , Jeff King , Andrzej Hunt , Andrzej Hunt Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Andrzej Hunt From: Andrzej Hunt ibuf can be reused for multiple iterations of the loop. Specifically: deflate() overwrites s.avail_in to show how much of the input buffer has not been processed yet - and sometimes leaves 'avail_in > 0', in which case ibuf will be processed again during the loop's subsequent iteration. But if we declare ibuf within the loop, then (in theory) we get a new (and uninitialised) buffer for every iteration. In practice, my compiler seems to resue the same buffer - meaning that this code does work - but it doesn't seem safe to rely on this behaviour. MSAN correctly catches this issue - as soon as we hit the 's.avail_in > 0' condition, we end up reading from what seems to be uninitialised memory. Therefore, we move ibuf out of the loop, making this reuse safe. See MSAN output from t1050-large below - the interesting part is the ibuf creation at the end, although there's a lot of indirection before we reach the read from unitialised memory: ==11294==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x7f75db58fb1c in crc32_little crc32.c:283:9 #1 0x7f75db58d5b3 in crc32_z crc32.c:220:20 #2 0x7f75db59668c in crc32 crc32.c:242:12 #3 0x8c94f8 in hashwrite csum-file.c:101:15 #4 0x825faf in stream_to_pack bulk-checkin.c:154:5 #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8 #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15 #7 0xa7cff2 in index_stream object-file.c:2234:9 #8 0xa7bff7 in index_fd object-file.c:2256:9 #9 0xa7d22d in index_path object-file.c:2274:7 #10 0xb3c8c9 in add_to_index read-cache.c:802:7 #11 0xb3e039 in add_file_to_index read-cache.c:835:9 #12 0x4a99c3 in add_files add.c:458:7 #13 0x4a7276 in cmd_add add.c:670:18 #14 0x4a1e76 in run_builtin git.c:461:11 #15 0x49e1e7 in handle_builtin git.c:714:3 #16 0x4a0c08 in run_argv git.c:781:4 #17 0x49d5a8 in cmd_main git.c:912:19 #18 0x7974da in main common-main.c:52:11 #19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349) #20 0x421bd9 in _start start.S:120 Uninitialized value was stored to memory at #0 0x7f75db58fa6b in crc32_little crc32.c:283:9 #1 0x7f75db58d5b3 in crc32_z crc32.c:220:20 #2 0x7f75db59668c in crc32 crc32.c:242:12 #3 0x8c94f8 in hashwrite csum-file.c:101:15 #4 0x825faf in stream_to_pack bulk-checkin.c:154:5 #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8 #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15 #7 0xa7cff2 in index_stream object-file.c:2234:9 #8 0xa7bff7 in index_fd object-file.c:2256:9 #9 0xa7d22d in index_path object-file.c:2274:7 #10 0xb3c8c9 in add_to_index read-cache.c:802:7 #11 0xb3e039 in add_file_to_index read-cache.c:835:9 #12 0x4a99c3 in add_files add.c:458:7 #13 0x4a7276 in cmd_add add.c:670:18 #14 0x4a1e76 in run_builtin git.c:461:11 #15 0x49e1e7 in handle_builtin git.c:714:3 #16 0x4a0c08 in run_argv git.c:781:4 #17 0x49d5a8 in cmd_main git.c:912:19 #18 0x7974da in main common-main.c:52:11 #19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349) Uninitialized value was stored to memory at #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3 #1 0x7f75db5c2011 in flush_pending deflate.c:746:5 #2 0x7f75db5cafa0 in deflate_stored deflate.c:1815:9 #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34 #4 0xd80b7f in git_deflate zlib.c:244:12 #5 0x825dff in stream_to_pack bulk-checkin.c:140:12 #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8 #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15 #8 0xa7cff2 in index_stream object-file.c:2234:9 #9 0xa7bff7 in index_fd object-file.c:2256:9 #10 0xa7d22d in index_path object-file.c:2274:7 #11 0xb3c8c9 in add_to_index read-cache.c:802:7 #12 0xb3e039 in add_file_to_index read-cache.c:835:9 #13 0x4a99c3 in add_files add.c:458:7 #14 0x4a7276 in cmd_add add.c:670:18 #15 0x4a1e76 in run_builtin git.c:461:11 #16 0x49e1e7 in handle_builtin git.c:714:3 #17 0x4a0c08 in run_argv git.c:781:4 #18 0x49d5a8 in cmd_main git.c:912:19 #19 0x7974da in main common-main.c:52:11 Uninitialized value was stored to memory at #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3 #1 0x7f75db644241 in _tr_stored_block trees.c:873:5 #2 0x7f75db5cad7c in deflate_stored deflate.c:1813:9 #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34 #4 0xd80b7f in git_deflate zlib.c:244:12 #5 0x825dff in stream_to_pack bulk-checkin.c:140:12 #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8 #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15 #8 0xa7cff2 in index_stream object-file.c:2234:9 #9 0xa7bff7 in index_fd object-file.c:2256:9 #10 0xa7d22d in index_path object-file.c:2274:7 #11 0xb3c8c9 in add_to_index read-cache.c:802:7 #12 0xb3e039 in add_file_to_index read-cache.c:835:9 #13 0x4a99c3 in add_files add.c:458:7 #14 0x4a7276 in cmd_add add.c:670:18 #15 0x4a1e76 in run_builtin git.c:461:11 #16 0x49e1e7 in handle_builtin git.c:714:3 #17 0x4a0c08 in run_argv git.c:781:4 #18 0x49d5a8 in cmd_main git.c:912:19 #19 0x7974da in main common-main.c:52:11 Uninitialized value was stored to memory at #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3 #1 0x7f75db5c8fcf in deflate_stored deflate.c:1783:9 #2 0x7f75db5bb7d2 in deflate deflate.c:1005:34 #3 0xd80b7f in git_deflate zlib.c:244:12 #4 0x825dff in stream_to_pack bulk-checkin.c:140:12 #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8 #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15 #7 0xa7cff2 in index_stream object-file.c:2234:9 #8 0xa7bff7 in index_fd object-file.c:2256:9 #9 0xa7d22d in index_path object-file.c:2274:7 #10 0xb3c8c9 in add_to_index read-cache.c:802:7 #11 0xb3e039 in add_file_to_index read-cache.c:835:9 #12 0x4a99c3 in add_files add.c:458:7 #13 0x4a7276 in cmd_add add.c:670:18 #14 0x4a1e76 in run_builtin git.c:461:11 #15 0x49e1e7 in handle_builtin git.c:714:3 #16 0x4a0c08 in run_argv git.c:781:4 #17 0x49d5a8 in cmd_main git.c:912:19 #18 0x7974da in main common-main.c:52:11 #19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349) Uninitialized value was stored to memory at #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3 #1 0x7f75db5ea545 in read_buf deflate.c:1181:5 #2 0x7f75db5c97f7 in deflate_stored deflate.c:1791:9 #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34 #4 0xd80b7f in git_deflate zlib.c:244:12 #5 0x825dff in stream_to_pack bulk-checkin.c:140:12 #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8 #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15 #8 0xa7cff2 in index_stream object-file.c:2234:9 #9 0xa7bff7 in index_fd object-file.c:2256:9 #10 0xa7d22d in index_path object-file.c:2274:7 #11 0xb3c8c9 in add_to_index read-cache.c:802:7 #12 0xb3e039 in add_file_to_index read-cache.c:835:9 #13 0x4a99c3 in add_files add.c:458:7 #14 0x4a7276 in cmd_add add.c:670:18 #15 0x4a1e76 in run_builtin git.c:461:11 #16 0x49e1e7 in handle_builtin git.c:714:3 #17 0x4a0c08 in run_argv git.c:781:4 #18 0x49d5a8 in cmd_main git.c:912:19 #19 0x7974da in main common-main.c:52:11 Uninitialized value was created by an allocation of 'ibuf' in the stack frame of function 'stream_to_pack' #0 0x825710 in stream_to_pack bulk-checkin.c:101 SUMMARY: MemorySanitizer: use-of-uninitialized-value crc32.c:283:9 in crc32_little Exiting Signed-off-by: Andrzej Hunt --- bulk-checkin.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bulk-checkin.c b/bulk-checkin.c index 127312acd1ed..b023d9959aae 100644 --- a/bulk-checkin.c +++ b/bulk-checkin.c @@ -100,6 +100,7 @@ static int stream_to_pack(struct bulk_checkin_state *state, const char *path, unsigned flags) { git_zstream s; + unsigned char ibuf[16384]; unsigned char obuf[16384]; unsigned hdrlen; int status = Z_OK; @@ -113,8 +114,6 @@ static int stream_to_pack(struct bulk_checkin_state *state, s.avail_out = sizeof(obuf) - hdrlen; while (status != Z_STREAM_END) { - unsigned char ibuf[16384]; - if (size && !s.avail_in) { ssize_t rsize = size < sizeof(ibuf) ? size : sizeof(ibuf); ssize_t read_result = read_in_full(fd, ibuf, rsize); From patchwork Mon Jun 14 15:51:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrzej Hunt X-Patchwork-Id: 12319277 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=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 35174C48BE6 for ; Mon, 14 Jun 2021 15:51:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 14B9461352 for ; Mon, 14 Jun 2021 15:51:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233917AbhFNPxY (ORCPT ); Mon, 14 Jun 2021 11:53:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46300 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233538AbhFNPxX (ORCPT ); Mon, 14 Jun 2021 11:53:23 -0400 Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A88E7C061767 for ; Mon, 14 Jun 2021 08:51:20 -0700 (PDT) Received: by mail-wr1-x436.google.com with SMTP id l2so15117846wrw.6 for ; Mon, 14 Jun 2021 08:51:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=4OHcT78GTIKQC/OACUpiZAP0UG5Wm9XiPwGfbqerTi8=; b=GyTWnRZzg04Ic1IrGJNxgOMSyf5tuyBd1dgIn+rdrZA18NxA52/QZvKVONt5F44dsQ 7/dGgorRI7SVwgp8dwMbah1so1MytHo6PmRuKV3OT8cwTnSTMJkk2LUyVwVJ6rbW/eah IpDtQoJ6UEt8UP/9vhwcTzbk2b6T3Keuk1G0ax6SOOMzEsIex7gt3/+HLwIQl4Kk5p7j nIj949FozQJ1ZB4e7YiUfIhIaryzXNiIy+HFGrGHrwa9QTR5JUkKCfpNXBtnE7kn7ZVD g9fw8YPXQvvSbrZD4myDGCgeoMP9Zl1mgwRs3LhvwiawN3ZUks6cjTriWTLmqMEkprjd mTMA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=4OHcT78GTIKQC/OACUpiZAP0UG5Wm9XiPwGfbqerTi8=; b=GVRdgy/g1X20prUNxZRZ2vA8U01MO1cp1QsHXiEZCkpOAsy+sTZae/EBlakMCch+sC yBnZxd0Un5zFuGi9IiQU5AIAbF70lxXKg+9dFy2m1vtpshrlZt8bk9itby134Co450rb tOt2NZ0Lnc0OPbD08uX7gwo5r/+if7bvTQgSJEZ5O3pwZQG8jL/3sukbcMvITUEZuUKB eTJpBQLsTVk3/GXvOqPq1AAnwlXLzkjufy+l5xckp7Q+pMAIhjsQyWzfBE1WE6c3TIZG AFFgBnMfYINybs/E2XBeirHsbkCPiBx+qHwpcJOLuNAqLbq6l4jnqy2NE7PQktnia98M hgww== X-Gm-Message-State: AOAM532bxi8AuQ2UJjJ2F88V4PeDAz+LmQ1f45tvyAzI46gVGB4g9O9N MkHdLJnVlgPI0BDZhO4xcC/s6yCiwWA= X-Google-Smtp-Source: ABdhPJzjhXopAMrsHpKFsSf7/pdmGsj9BEAmXg+0djN/45333I2fopcIS1hAnk/51IpvWFS4Lrlx6g== X-Received: by 2002:adf:e7d0:: with SMTP id e16mr19113904wrn.202.1623685879352; Mon, 14 Jun 2021 08:51:19 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id n18sm14890162wmq.41.2021.06.14.08.51.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Jun 2021 08:51:19 -0700 (PDT) Message-Id: <6943eb511bee8323eb65f1466634ce9307694796.1623685877.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Mon, 14 Jun 2021 15:51:15 +0000 Subject: [PATCH v2 2/3] split-index: use oideq instead of memcmp to compare object_id's Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Chris Torek , Jeff King , Andrzej Hunt , Andrzej Hunt Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Andrzej Hunt From: Andrzej Hunt cache_entry contains an object_id, and compare_ce_content() would include that field when calling memcmp on a subset of the cache_entry. Depending on which hashing algorithm is being used, only part of object_id.hash is actually being used, therefore including it in a memcmp() is incorrect. Instead we choose to exclude the object_id when calling memcmp(), and call oideq() separately. This issue was found when running t1700-split-index with MSAN, see MSAN output below (on my machine, offset 76 corresponds to 4 bytes after the start of object_id.hash). Uninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92) ==27914==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 #1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8 #2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9 #3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2 #4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8 #5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7 #6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9 #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11 #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3 #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4 #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19 #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11 #12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349) #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120 Uninitialized value was stored to memory at #0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3 #1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2 #2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18 #3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2 #4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11 #5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12 #6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12 #7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6 #8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17 #9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9 #10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8 #11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7 #12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2 #13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12 #14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9 #15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11 #16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3 #17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4 #18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19 #19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11 Uninitialized value was created by a heap allocation #0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3 #1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8 #2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9 #3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6 #4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3 #5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c #6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17 #7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8 #8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8 #9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2 #10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6 #11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11 #12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3 #13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4 #14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19 #15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11 #16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp Exiting Signed-off-by: Andrzej Hunt --- split-index.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/split-index.c b/split-index.c index 4d6e52d46f75..8e52e891c3bc 100644 --- a/split-index.c +++ b/split-index.c @@ -207,7 +207,8 @@ static int compare_ce_content(struct cache_entry *a, struct cache_entry *b) b->ce_flags &= ondisk_flags; ret = memcmp(&a->ce_stat_data, &b->ce_stat_data, offsetof(struct cache_entry, name) - - offsetof(struct cache_entry, ce_stat_data)); + offsetof(struct cache_entry, oid)) || + !oideq(&a->oid, &b->oid); a->ce_flags = ce_flags; b->ce_flags = base_flags; From patchwork Mon Jun 14 15:51:16 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrzej Hunt X-Patchwork-Id: 12319279 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=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 2FB7EC2B9F4 for ; Mon, 14 Jun 2021 15:51:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 14CC961378 for ; Mon, 14 Jun 2021 15:51:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233933AbhFNPx0 (ORCPT ); Mon, 14 Jun 2021 11:53:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46302 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233152AbhFNPxY (ORCPT ); Mon, 14 Jun 2021 11:53:24 -0400 Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 29C6CC061574 for ; Mon, 14 Jun 2021 08:51:21 -0700 (PDT) Received: by mail-wm1-x32b.google.com with SMTP id h22-20020a05600c3516b02901a826f84095so253075wmq.5 for ; Mon, 14 Jun 2021 08:51:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=6+FQAr3QFN14/bbpW/c2KgXaNuTk9/pA79fJdM5xfAY=; b=aeEHJfKLYSRrlNnd+hD9gmdn9LjZxCKiZt9hyKTLq9F4VsaoQH3i+eM++prxg9KH5P s26TQl9dH368Y/LLBH+6fLP+Zsm7jBJlHxGzdOzKv/dGbnSirEmUCP9p0mAtvUPxYphA nsEinVgmuKy26Wdk1MKg0uUDqfo7HBEoGBoFOassxkgsBzPtXuoSu6w6wnUMxqXbD5Ws Y2yBFzBLLLxXTM2OiBtQZl/yFSxiM8XG5EEcvoImx0lS083kfrjV1GFYMEJUtuIMtExc 2T4xd/Z1IdEdyPFhT0IyxkGeJQg77123KMfCqWNmfKHRngHkStkea18ybFw0eXjW5ZrU avIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=6+FQAr3QFN14/bbpW/c2KgXaNuTk9/pA79fJdM5xfAY=; b=i/n2f0wIZugF+xGwpfWCfFFpyZ4Hcakt4643suHZc3e9ioj5hfDjukRvMaQSPSdAnU hAqn7k5KDJv+gMed9YLDXhVHOFo/OplM5Cj6YaP6Lju5I62hgOLi258Vo1QT13VD+MIo vUFEQgDOBgAC8i79+1mzKaguU6qxW14yQUTuFSyD6I6BG6cMa7xLqux6dLRchE5WNCsi wcnBbsnpXxQJXWh+D7lQK40EdUXQBRwwyOILlikE7n+G5wfPxGigbmqhuoGgiDDJKcTc 6slDK0cP3tp3lxLJwcbpl80UmO2Ag0M9RtWokHCT4kRP6A9qaj7kBJ/FR/2RwJzU7lPf CHgg== X-Gm-Message-State: AOAM532YDYGWvAa2sAqbweFpXU4w2Im9pWtfg8ugvb2w9uUOiUNC37Vm BlFxtxpQO/fFi9ywkWigEM0imOGIQuc= X-Google-Smtp-Source: ABdhPJyPRNm1VcrL18a/bgbhMJlUR9JE/7pvL0Qjr4M4O8zo149h+HKWUeZ601UkuiVjmN17x9N5Pg== X-Received: by 2002:a1c:e90d:: with SMTP id q13mr17178570wmc.163.1623685879851; Mon, 14 Jun 2021 08:51:19 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id n10sm15447211wri.77.2021.06.14.08.51.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Jun 2021 08:51:19 -0700 (PDT) Message-Id: <4bdc0b77f6f2a379c229b221460558ea8def5ad1.1623685877.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Mon, 14 Jun 2021 15:51:16 +0000 Subject: [PATCH v2 3/3] builtin/checkout--worker: zero-initialise struct to avoid MSAN complaints Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Chris Torek , Jeff King , Andrzej Hunt , Andrzej Hunt Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Andrzej Hunt From: Andrzej Hunt report_result() sends a struct to the parent process, but that struct would contain uninitialised padding bytes. Running this code under MSAN rightly triggers a warning - but we don't particularly care about this warning because we control the receiving code, and we therefore know that those padding bytes won't be read on the receiving end. We could simply suppress this warning under MSAN with the approporiate ifdef'd attributes, but a less intrusive solution is to 0-initialise the struct, which guarantees that the padding will also be initialised. Interestingly, in the error-case branch, we only try to copy the first two members of pc_item_result, by copying only PC_ITEM_RESULT_BASE_SIZE bytes. However PC_ITEM_RESULT_BASE_SIZE is defined as 'offsetof(the_last_member)', which means that we're copying padding bytes after the end of the second last member. We could avoid doing this by redefining PC_ITEM_RESULT_BASE_SIZE as 'offsetof(second_last_member) + sizeof(second_last_member)', but there's no huge benefit to doing so (and this patch silences the MSAN warning in this scenario either way). MSAN output from t2080 (partially interleaved due to the parallel work :) ): Uninitialized bytes in __interceptor_write at offset 12 inside [0x7fff37d83408, 160) ==23279==WARNING: MemorySanitizer: use-of-uninitialized-value Uninitialized bytes in __interceptor_write at offset 12 inside [0x7ffdb8a07ec8, 160) ==23280==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0xd5ac28 in xwrite /home/ahunt/git/git/wrapper.c:256:8 #1 0xd5b327 in write_in_full /home/ahunt/git/git/wrapper.c:311:21 #2 0xb0a8c4 in do_packet_write /home/ahunt/git/git/pkt-line.c:221:6 #3 0xb0a5fd in packet_write /home/ahunt/git/git/pkt-line.c:242:6 #4 0x4f7441 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:69:2 #5 0x4f6be6 in worker_loop /home/ahunt/git/git/builtin/checkout--worker.c:100:3 #6 0x4f68d3 in cmd_checkout__worker /home/ahunt/git/git/builtin/checkout--worker.c:143:2 #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11 #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3 #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4 #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19 #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11 #12 0x7f8778114349 in __libc_start_main (/lib64/libc.so.6+0x24349) #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120 Uninitialized value was created by an allocation of 'res' in the stack frame of function 'report_result' #0 0x4f72c0 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:55 SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/ahunt/git/git/wrapper.c:256:8 in xwrite Exiting #0 0xd5ac28 in xwrite /home/ahunt/git/git/wrapper.c:256:8 #1 0xd5b327 in write_in_full /home/ahunt/git/git/wrapper.c:311:21 #2 0xb0a8c4 in do_packet_write /home/ahunt/git/git/pkt-line.c:221:6 #3 0xb0a5fd in packet_write /home/ahunt/git/git/pkt-line.c:242:6 #4 0x4f7441 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:69:2 #5 0x4f6be6 in worker_loop /home/ahunt/git/git/builtin/checkout--worker.c:100:3 #6 0x4f68d3 in cmd_checkout__worker /home/ahunt/git/git/builtin/checkout--worker.c:143:2 #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11 #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3 #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4 #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19 #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11 #12 0x7f2749a0e349 in __libc_start_main (/lib64/libc.so.6+0x24349) #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120 Uninitialized value was created by an allocation of 'res' in the stack frame of function 'report_result' #0 0x4f72c0 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:55 SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/ahunt/git/git/wrapper.c:256:8 in xwrite Signed-off-by: Andrzej Hunt --- builtin/checkout--worker.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/checkout--worker.c b/builtin/checkout--worker.c index 289a9b8f89d0..fb9fd13b73c4 100644 --- a/builtin/checkout--worker.c +++ b/builtin/checkout--worker.c @@ -53,7 +53,7 @@ static void packet_to_pc_item(const char *buffer, int len, static void report_result(struct parallel_checkout_item *pc_item) { - struct pc_item_result res; + struct pc_item_result res = { 0 }; size_t size; res.id = pc_item->id;