From patchwork Sat Sep 18 13:49:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrzej Hunt X-Patchwork-Id: 12503863 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 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 D0B14C433F5 for ; Sat, 18 Sep 2021 13:49:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8595561368 for ; Sat, 18 Sep 2021 13:49:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237853AbhIRNvF (ORCPT ); Sat, 18 Sep 2021 09:51:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35996 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237615AbhIRNvF (ORCPT ); Sat, 18 Sep 2021 09:51:05 -0400 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 651FAC061574 for ; Sat, 18 Sep 2021 06:49:41 -0700 (PDT) Received: by mail-wm1-x332.google.com with SMTP id c8-20020a7bc008000000b002e6e462e95fso12091451wmb.2 for ; Sat, 18 Sep 2021 06:49:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=KdxOJyRC8jIGOcUwf033kqY1zKu7y6LeQAXcMpFz74o=; b=XZgE5Jdyg+whJsSPNnG7chqumHwApcE+4UvS2/wf9DaPzK0RAeQDiUGADkNMDuyB2K h1k/FgCp7pbzlpTJ8q+jcDxXprJlmktovhfXkraDvcDpFluK2oOV2M3xROlOf9kpH1vK dFbI0aJVON+iYZWrXQmwlHd412DuOV5bMm7DJhvW6dfuKkgOAJbFTbmrvdE/BB5vjMvP IcwSOVjrmvMMh5QQ8/7qJaT3FmcSIvpcoYtKgJcFhAgoBW/sSw4Jz9XUH+vHSoVcgduF exEkoHYnYvGWUFS04fDs0E2t65oS0TUTwAy8VBrdkiTTCrLZT8PH8o43ZsxvX/PkkdWy B5AA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=KdxOJyRC8jIGOcUwf033kqY1zKu7y6LeQAXcMpFz74o=; b=W/agYstxPWp84tZ0DoeMokC0aV+mIpaHXDVsBnbaJ3Jo8ukdMUxC3L+nq/6zL5vF0Y qiSVCBQxKQCKsG8QNjJVY3tA9E0gHrJGjfdXjKrr85Qjqt7oa4dPny9RsAP6yLbqhte+ O8BDECe2pASiOi0PJJ44pJpU7tz84S18n/By2Po3poov3RVndFEW4CwJbvKpjfq9ngsR 47ZO/7n88oKXkGXydkyr4RbQzzb1f+CDqoBSa/wQ9coPobjn25/9nHQ51mI57MIvsPIt +MKd1EJ2S+ehPR20Dt6QhAjDKqJjCeXzUSCsK282ijzfpWchvt0s7qcNiRqaqMIcRc1F P4KQ== X-Gm-Message-State: AOAM5326u1v+omJQ6++jhXMh78NB10b73xqRCafIYsZyWAFrZ7Z3Bfjk tpmhMrhttSqmajp8MueAD31h4yV9hbA= X-Google-Smtp-Source: ABdhPJw66FoyXPzcMlbPrUVQspkdEdgdTlAW8UTgsPdwvdngf/vfp3NjMmAMBPa4rJ3509DIQ+YkYg== X-Received: by 2002:a05:600c:245:: with SMTP id 5mr20566946wmj.53.1631972980073; Sat, 18 Sep 2021 06:49:40 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id k4sm9435609wmj.30.2021.09.18.06.49.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Sep 2021 06:49:39 -0700 (PDT) Message-Id: <6d54bc264e2f9ce519f32c0673167a00bab55573.1631972978.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Sat, 18 Sep 2021 13:49:37 +0000 Subject: [PATCH 1/2] log: UNLEAK rev to silence a large number of leaks Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Carlo Arenas , Andrzej Hunt , Andrzej Hunt Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Andrzej Hunt From: Andrzej Hunt cmd_show puts a lot of data into rev, and doesn't clean it up before returning. That's reasonable - we use most if not all of rev up until cmd_show is finished - there's not much value in doing a proper cleanup. Therefore we take the easy way out and UNLEAK rev. The UNLEAK has to be performed early on, as cmd_show might return via cmd_log_walk() in the next few lines, or it might continue to the no-walk implementation below. This patch silences the following leaks which were found when running t0000 against LSAN: Direct leak of 41 byte(s) in 1 object(s) allocated from: #0 0x486834 in strdup /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3 #1 0x9ab168 in xstrdup /home/ahunt/oss-fuzz/git/wrapper.c:29:14 #2 0x83cced in add_object_array_with_path /home/ahunt/oss-fuzz/git/object.c:349:17 #3 0x8f4f5a in add_pending_object_with_path /home/ahunt/oss-fuzz/git/revision.c:329:2 #4 0x8eb2b6 in handle_revision_arg_1 /home/ahunt/oss-fuzz/git/revision.c:2082:2 #5 0x8eadad in handle_revision_arg /home/ahunt/oss-fuzz/git/revision.c:2089:12 #6 0x8eea99 in setup_revisions /home/ahunt/oss-fuzz/git/revision.c:2756:7 #7 0x59c024 in cmd_log_init_finish /home/ahunt/oss-fuzz/git/builtin/log.c:206:9 #8 0x5998d8 in cmd_log_init /home/ahunt/oss-fuzz/git/builtin/log.c:275:2 #9 0x599f9b in cmd_show /home/ahunt/oss-fuzz/git/builtin/log.c:641:2 #10 0x4cd92d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 #11 0x4cb5fa in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 #12 0x4ccf57 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 #13 0x4caf49 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 #14 0x69ce3e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 #15 0x7f7c56197349 in __libc_start_main (/lib64/libc.so.6+0x24349) Direct leak of 32 byte(s) in 1 object(s) allocated from: #0 0x49a9d2 in calloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3 #1 0x9ab4c2 in xcalloc /home/ahunt/oss-fuzz/git/wrapper.c:140:8 #2 0x59c269 in cmd_log_init_finish /home/ahunt/oss-fuzz/git/builtin/log.c:233:18 #3 0x5998d8 in cmd_log_init /home/ahunt/oss-fuzz/git/builtin/log.c:275:2 #4 0x599f9b in cmd_show /home/ahunt/oss-fuzz/git/builtin/log.c:641:2 #5 0x4cd92d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 #6 0x4cb5fa in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 #7 0x4ccf57 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 #8 0x4caf49 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 #9 0x69ce3e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 #10 0x7f7c56197349 in __libc_start_main (/lib64/libc.so.6+0x24349) Indirect leak of 41 byte(s) in 1 object(s) allocated from: #0 0x486834 in strdup /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3 #1 0x9ab168 in xstrdup /home/ahunt/oss-fuzz/git/wrapper.c:29:14 #2 0x8f5e30 in add_rev_cmdline /home/ahunt/oss-fuzz/git/revision.c:1482:23 #3 0x8eb26d in handle_revision_arg_1 /home/ahunt/oss-fuzz/git/revision.c:2081:2 #4 0x8eadad in handle_revision_arg /home/ahunt/oss-fuzz/git/revision.c:2089:12 #5 0x8eea99 in setup_revisions /home/ahunt/oss-fuzz/git/revision.c:2756:7 #6 0x59c024 in cmd_log_init_finish /home/ahunt/oss-fuzz/git/builtin/log.c:206:9 #7 0x5998d8 in cmd_log_init /home/ahunt/oss-fuzz/git/builtin/log.c:275:2 #8 0x599f9b in cmd_show /home/ahunt/oss-fuzz/git/builtin/log.c:641:2 #9 0x4cd92d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 #10 0x4cb5fa in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 #11 0x4ccf57 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 #12 0x4caf49 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 #13 0x69ce3e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 #14 0x7fc4b3f06349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt Signed-off-by: Carlo Marcelo Arenas Belón Signed-off-by: Carlo Marcelo Arenas Belón Signed-off-by: Carlo Marcelo Arenas Belón --- builtin/log.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/log.c b/builtin/log.c index f75d87e8d7f..6faaddf17a6 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -644,6 +644,7 @@ int cmd_show(int argc, const char **argv, const char *prefix) opt.def = "HEAD"; opt.tweak = show_setup_revisions_tweak; cmd_log_init(argc, argv, prefix, &rev, &opt); + UNLEAK(rev); if (!rev.no_walk) return cmd_log_walk(&rev); From patchwork Sat Sep 18 13:49:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrzej Hunt X-Patchwork-Id: 12503865 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 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 0AE63C433EF for ; Sat, 18 Sep 2021 13:49:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E674E61050 for ; Sat, 18 Sep 2021 13:49:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238215AbhIRNvH (ORCPT ); Sat, 18 Sep 2021 09:51:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35998 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237617AbhIRNvF (ORCPT ); Sat, 18 Sep 2021 09:51:05 -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 2088FC061574 for ; Sat, 18 Sep 2021 06:49:42 -0700 (PDT) Received: by mail-wr1-x42b.google.com with SMTP id d6so19844077wrc.11 for ; Sat, 18 Sep 2021 06:49:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=SjrbHt7eGrvXHaiVQoc9XYQRsVwlQl3vizI1S/fC2LI=; b=d3zPHqAm+5WyrJ3INYDQ6TUyRbxLgOiLTZ5FIKZbpvP1FEmucXJSpRT9qESEtmwPVI g77omFKBVfshKiOhIi2NsXiK0ASHPaKiwy0WcBzNUZaaAb/Mjau3ZEWzT1scE1w9CJlO g2Hz70TG5l+sxSo/sGFVb54LkncxnXYHLfMjt9Dk+P0ssj8htCmzBaSMmzK93rUY6X9z 2dA3Z+Eys+rZaBAKwu4PhzBYlcN4aThoo1uzFUuNGVJbomOCtvxD5ilhjqDpn2iDog1h zbGWf3Q53Gj6bdgL6pAGSJBKId24ZZyCjQIINXrdg7WFVpWWvI0F7aDT3X0mEturfhjv MS6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=SjrbHt7eGrvXHaiVQoc9XYQRsVwlQl3vizI1S/fC2LI=; b=edhwmt2irhkvfZY8uX3hI+/ZRQYDxTWnrWS1utNPcJRsR7JtZ5A/Kx3vuRgV7g7T5C L9BWpIOF8xDuenaAeHzC+9uPw6Z6i2uiYm3u6ok1nWqiVLXrayQ1GYcjrvJtgXP2Gq9c z26tRlFfOKF1vrL9iSZnDAENNQQg9bRNwINTFxvV4kmHWKqQtAWMeiIkqBlSZiwEPyzN lPKCXxnqy3HfYlq/cZvjYCqKzZ/D77s5GzzF/3ZZgyjsiXcBlOSJOiz02+Xn5a3qRg89 3rlJoxoRqTHseEhqr3Z84o8D6V3UO5gYtLJkMdkcRlo/GT9yGVv5VCFfu++PDhMui0Mm LwaQ== X-Gm-Message-State: AOAM533fg2qEnlstfnCBJkNma8DTw2fxhkgk70B33UPxSj/fFCZtEc+V 6NIUJhmlSEV8EEhYKtxXMebgdzL3xoo= X-Google-Smtp-Source: ABdhPJwk+8YR5IwIcESGPdDVM4d5ct2/t5ACupGHI76mwuyGGUYiI0TJNWtEdrBErKnqcIs/Fz1nZA== X-Received: by 2002:a5d:6741:: with SMTP id l1mr18208110wrw.289.1631972980677; Sat, 18 Sep 2021 06:49:40 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id z17sm9102518wml.24.2021.09.18.06.49.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Sep 2021 06:49:40 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Sat, 18 Sep 2021 13:49:38 +0000 Subject: [PATCH 2/2] log: UNLEAK original pending objects Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Carlo Arenas , Andrzej Hunt , Andrzej Hunt Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Andrzej Hunt From: Andrzej Hunt cmd_show() uses objects to point to rev.pending's objects. Later, it might detach rev.pending's objects: when handling an OBJ_COMMIT it will reset rev.pending (without freeing its objects). Detaching (as opposed to freeing) is necessary because cmd_show() continues iterating over the original objects array. We choose to UNLEAK because there's no real advantage to cleaning up properly (cmd_show() exits immediately after looping over these objects). A number of alternatives exist, but are all significantly more complex for no gain: Alternative 1: Convert objects into an object_array, and memcpy rev.pending into it (followed by detaching rev.pending immediately - making objects the owner of what used to be rev.pending). Then we could safely objects_array_clear() at the end of cmd_show(). And we can rely on a preexisting UNLEAK(rev) to avoid having to clean up rev.pending. This is a more complex and riskier approach vs a simple UNLEAK, and doesn't add any user-visible value. Alternative 2: A variation on alternative 1. We make objects own the object_array as before. Once we're done, we free the new rev.pending array (which might be empty), and we memcpy objects back into rev.pending, relying on the existin UNLEAK(rev) to avoid having to free rev.pending. ASAN output from t0000: Direct leak of 41 byte(s) in 1 object(s) allocated from: #0 0x487504 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:437:3 #1 0x9e4ef8 in xstrdup wrapper.c:29:14 #2 0x86395d in add_object_array_with_path object.c:366:17 #3 0x9264fc in add_pending_object_with_path revision.c:330:2 #4 0x91c4e0 in handle_revision_arg_1 revision.c:2086:2 #5 0x91bfcd in handle_revision_arg revision.c:2093:12 #6 0x91ff5a in setup_revisions revision.c:2780:7 #7 0x5a7678 in cmd_log_init_finish builtin/log.c:206:9 #8 0x5a4f18 in cmd_log_init builtin/log.c:278:2 #9 0x5a55d1 in cmd_show builtin/log.c:646:2 #10 0x4cff30 in run_builtin git.c:461:11 #11 0x4cdb00 in handle_builtin git.c:713:3 #12 0x4cf527 in run_argv git.c:780:4 #13 0x4cd426 in cmd_main git.c:911:19 #14 0x6b2eb5 in main common-main.c:52:11 #15 0x7f74fc9bd349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 41 byte(s) leaked in 1 allocation(s). Signed-off-by: Andrzej Hunt --- builtin/log.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/log.c b/builtin/log.c index 6faaddf17a6..769ee6a9258 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -702,7 +702,8 @@ int cmd_show(int argc, const char **argv, const char *prefix) ret = error(_("unknown type: %d"), o->type); } } - free(objects); + UNLEAK(objects); + return ret; }