From patchwork Wed Sep 1 02:06:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12468037 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=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,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 D7752C432BE for ; Wed, 1 Sep 2021 02:06:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B364560F56 for ; Wed, 1 Sep 2021 02:06:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241015AbhIACH1 (ORCPT ); Tue, 31 Aug 2021 22:07:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55506 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230333AbhIACHY (ORCPT ); Tue, 31 Aug 2021 22:07:24 -0400 Received: from mail-il1-x12f.google.com (mail-il1-x12f.google.com [IPv6:2607:f8b0:4864:20::12f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4796FC061575 for ; Tue, 31 Aug 2021 19:06:28 -0700 (PDT) Received: by mail-il1-x12f.google.com with SMTP id r6so1728351ilt.13 for ; Tue, 31 Aug 2021 19:06:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=OIE3VXFxWnkGYSPaughoSk1zvDBancJLezU+Bont/ks=; b=O5GtY+iJOpdJnk8VnHMndAB1u2l+l0zneXbfFDpBmdZ1BT4Ya8AEqByb8XJDIoMzGW RoXRkbOwqoyw8ZUYoWfZTogsrAJxUBGBqmSKOOtANkMKqLYQVyWgtlvwlDb+TAeLtt0V pf2OL3SppwmzgILfN8JgRQpqtu+PRLVXc62wSJ2YvJV0p+SqS64tkU2vFtHnwugpooqY /vKdMEXaC585BNZOqQYhu+mmXtPDTqIUDiWan70/dQMVHn1+ShyRD1GTf2FRGKpdcDnC NFyss0yUsXOlu88B0Gkw5Q2qCbbWAoJAHAXceOJxeFFu1n3DVquyak87oYH/4o87Ug8M u2PQ== 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; bh=OIE3VXFxWnkGYSPaughoSk1zvDBancJLezU+Bont/ks=; b=iPGi6FtedI6j30ft59a+CDpCjry1JiqA6FWxKZM0uMXYP0ZgbuV2r4Nh4mjDI2c77D /p+J9FlioGhMz9ps+eTcIXk1QhoCeJ2dvEJpf8YqrVZXQYJDzHIooFd6934qiZ9lqnMm /jVcvbqAfgLv5GvNBbicm15cfwsJsSzf0pjmKn7x1XJMxH6W/ySGjxf7xqn/KrqUjx27 f+e7FluTlYMkcivJTlXeG23Zz80iJFj7MXxFJY1h9zFdB9nwCEs2X53+S5m4m7/SlAlp ZQOyK9bJcffkh6Apgwh0AVKr7Z50rY8MfH/IYDAAt+JxRo7b07WIlZEhlHsmFMEdCIYK zrnw== X-Gm-Message-State: AOAM532GhVhAtNfRG22xH+d5kf0ymUbHmMFb+8bQh6zHHi34OHe/gvw9 Z2WPM3ksBkLaJomMJEz2kEbTM2ES6T7DuUHt X-Google-Smtp-Source: ABdhPJwLR7BaQqZKlzDwaWbKYVBvdvtqYkaobaHiTf369zEiXnHCsapsAc2SRRmdTA7U2HcfFvAHvA== X-Received: by 2002:a05:6e02:1a0e:: with SMTP id s14mr22098426ild.48.1630461987516; Tue, 31 Aug 2021 19:06:27 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id k14sm11048824ili.19.2021.08.31.19.06.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 Aug 2021 19:06:27 -0700 (PDT) Date: Tue, 31 Aug 2021 22:06:26 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net Subject: [PATCH 1/2] pack-write.c: rename `.idx` file into place last Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org We treat the presence of an `.idx` file as the indicator of whether or not it's safe to use a packfile. But `finish_tmp_packfile()` (which is responsible for writing the pack and moving the temporary versions of all of its auxiliary files into place) is inconsistent about the write order. But the `.rev` file is moved into place after the `.idx`, so it's possible for a process to open a pack in between the two (i.e., while the `.idx` file is in place but the `.rev` file is not) and mistakenly fall back to generating the pack's reverse index in memory. Though racy, this amounts to an unnecessary slow-down at worst, and doesn't affect the correctness of the resulting reverse index. Close this race by moving the .rev file into place before moving the .idx file into place. While we're here, only call strbuf_setlen() if we actually modified the buffer by bringing it inside of the same if-statement that calls rename(). Signed-off-by: Taylor Blau --- The diff is a little inscrutable here, since it shows the .idx hunk moving below the .rev hunk (instead of the .rev hunk moving above the .idx one), but the end-result is the same. pack-write.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) -- 2.33.0.96.g73915697e6 diff --git a/pack-write.c b/pack-write.c index f1fc3ecafa..277c60165e 100644 --- a/pack-write.c +++ b/pack-write.c @@ -490,18 +490,18 @@ void finish_tmp_packfile(struct strbuf *name_buffer, strbuf_setlen(name_buffer, basename_len); - strbuf_addf(name_buffer, "%s.idx", hash_to_hex(hash)); - if (rename(idx_tmp_name, name_buffer->buf)) - die_errno("unable to rename temporary index file"); - - strbuf_setlen(name_buffer, basename_len); - if (rev_tmp_name) { strbuf_addf(name_buffer, "%s.rev", hash_to_hex(hash)); if (rename(rev_tmp_name, name_buffer->buf)) die_errno("unable to rename temporary reverse-index file"); + + strbuf_setlen(name_buffer, basename_len); } + strbuf_addf(name_buffer, "%s.idx", hash_to_hex(hash)); + if (rename(idx_tmp_name, name_buffer->buf)) + die_errno("unable to rename temporary index file"); + strbuf_setlen(name_buffer, basename_len); free((void *)idx_tmp_name); From patchwork Wed Sep 1 02:06:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12468039 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=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,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 00367C4320A for ; Wed, 1 Sep 2021 02:06:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D03DD61075 for ; Wed, 1 Sep 2021 02:06:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241209AbhIACH2 (ORCPT ); Tue, 31 Aug 2021 22:07:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55518 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241020AbhIACH0 (ORCPT ); Tue, 31 Aug 2021 22:07:26 -0400 Received: from mail-io1-xd2b.google.com (mail-io1-xd2b.google.com [IPv6:2607:f8b0:4864:20::d2b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EDBB8C061575 for ; Tue, 31 Aug 2021 19:06:30 -0700 (PDT) Received: by mail-io1-xd2b.google.com with SMTP id a15so2149800iot.2 for ; Tue, 31 Aug 2021 19:06:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=1VxA8eA24M/VaCbj2GyZd23biCP9gqVmYOjFWdRdvVM=; b=RIr2O5NmWbMxzvG9HxYrtJSrEyVOwJS0NMmsSqfNVKbrIZQzElpFF4WEIHJijwq97p j7xISuFnaERoe9RVFxtMLUB3CCmCQgxbIh9seEsaCMp4aF7Jm845gwyuZvIre+hQEnz7 w/G2wz0rPr2x7ukK4v+x2hQDvxO3Td6dffIQHSM5HdNf8aNTrHcJOBn/q8QroM8JSjXv DkX64Rn9i8LlFvhDhGEj7a44rElKPxBzQvJad09IczDDn7MFW52yh2n1z5YlVvOEXZj1 /Vc+VVHx1bvI7/xzpWa3NFP+ksaPI/a9QO+JQuePLD27aVFuXiCQ0jr4GkADSsbQIKKJ nhIg== 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; bh=1VxA8eA24M/VaCbj2GyZd23biCP9gqVmYOjFWdRdvVM=; b=rytuB6rKOyo+gPXDHtnN+wTE1qnlHK6SwYMzXU8Ti+HodGN2LcbiJIUxJNBvQ35LXI RtBbT6zBj0mCmI8RVAJz6APXOOh6EJR068QtOrFGI2Gjg1Rp90Z9L06s14KgT/tDpioa vXiTFcO9dQ2bOKLvh398l2wodHxRZT+92DpfzRT3iKsSITK9WyrUxYo9BzENtvH1PuGn 9kGVOTIamkaCu6sFvJwjX4wjEtxiZn7b4gAOLzBGewecFYBgrhSIp2D+srsRUibvRcGF V4Wa6Kd4SF62f+BoBA3uXIg+y349ZCIAGYo9Dd1JJp/L3QuP6qrZZvPgxn9Zl+tfiWrk CxTg== X-Gm-Message-State: AOAM530ZloHcybCGael7UCcjwsLWSDgN27dDnYnIvxOmmSqfp3wflZNW GVB1EgtDmkEJgHl3nGtwKrUbsOwrD9iKcHOW X-Google-Smtp-Source: ABdhPJzz7z7QsXtLR9AIJVAail3jckj+g9TC1GTbEAJLpmF3yFm/fS26GbtbcW+f+jqvo2vZ9trNng== X-Received: by 2002:a05:6602:3c5:: with SMTP id g5mr24886264iov.42.1630461990271; Tue, 31 Aug 2021 19:06:30 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id t15sm10971509ilq.88.2021.08.31.19.06.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 Aug 2021 19:06:30 -0700 (PDT) Date: Tue, 31 Aug 2021 22:06:29 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net Subject: [PATCH 2/2] builtin/repack.c: move `.idx` files into place last Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In a similar spirit as the previous patch, fix the identical problem from `git repack` (which invokes `pack-objects` with a temporary location for output, and then moves the files into their final locations itself). Signed-off-by: Taylor Blau --- builtin/repack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/repack.c b/builtin/repack.c index 5f9bc74adc..c3e4771609 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -208,10 +208,10 @@ static struct { unsigned optional:1; } exts[] = { {".pack"}, - {".idx"}, {".rev", 1}, {".bitmap", 1}, {".promisor", 1}, + {".idx"}, }; static unsigned populate_pack_exts(char *name)