From patchwork Sat Jul 20 22:09:12 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13737920 Received: from pb-smtp2.pobox.com (pb-smtp2.pobox.com [64.147.108.71]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E8B1912EBE1 for ; Sat, 20 Jul 2024 22:09:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.108.71 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721513366; cv=none; b=aTaI3RqWR5+f77pobBw/ASjngZFggBEqnq3ElU5MoIbF4IM1k4zFgYblYzDy2ra81WPScNuhiRfwZNn59jyZqsbZ3iL3969YFJehBf0KuyuNT7FZlDnqR9N0/bH2f6qozfXhVaB316DerIa00Zelc2es0lo6vPUEEZyJZSjFlGY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721513366; c=relaxed/simple; bh=ZfNlbQ/aHfNyP4W7LaHcZV2R1tvK4+t0gIQoyHjgObk=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=PqonyqSZu1uROeiMk7t5G0XEHa+VdWPtbWTtScAh9cJFWOysJ6wkJAXw+Pi+wc6gCnJ4U03A+uTwuMDvS2EcTTYvhXNXh4L9URpt6FJNjzlaUq3+YaMxtvAYsDHAVf5bPfiTcRhphpLSWsQgCQK3cKc5N4jMC6p6mfv1C6M++VY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=aDEXJxhl; arc=none smtp.client-ip=64.147.108.71 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="aDEXJxhl" Received: from pb-smtp2.pobox.com (unknown [127.0.0.1]) by pb-smtp2.pobox.com (Postfix) with ESMTP id C1ADA1E864; Sat, 20 Jul 2024 18:09:18 -0400 (EDT) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; s=sasl; bh=ZfNlbQ/aHfNyP4W7LaHcZV2R1 tvK4+t0gIQoyHjgObk=; b=aDEXJxhl3fGi7+figiGjk1U0SoIfIRpXD70lYgZXY 9QX4dhgjLLzw1FVh287P+HRVk9CCKZHRFe/mIjYoUADuzaJVEW1tiS+5pQENzr4W 7jAkvMFHFIT81fqThiAvz3tk8aJXnaGhjKqcT41Jmenipwah7xHHsNHczcwAbte6 ig= Received: from pb-smtp2.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp2.pobox.com (Postfix) with ESMTP id B98B71E863; Sat, 20 Jul 2024 18:09:18 -0400 (EDT) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.139.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp2.pobox.com (Postfix) with ESMTPSA id 27AC01E862; Sat, 20 Jul 2024 18:09:18 -0400 (EDT) (envelope-from gitster@pobox.com) From: Junio C Hamano To: git@vger.kernel.org Cc: Phillip Wood Subject: [PATCH 1/2] safe.directory: normalize the checked path Date: Sat, 20 Jul 2024 15:09:12 -0700 Message-ID: <20240720220915.2933266-2-gitster@pobox.com> X-Mailer: git-send-email 2.46.0-rc1-48-g0900f1888e In-Reply-To: <20240720220915.2933266-1-gitster@pobox.com> References: <20240720220915.2933266-1-gitster@pobox.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: B568A3CC-46E4-11EF-9661-BAC1940A682E-77302942!pb-smtp2.pobox.com The pathname of a repository comes from getcwd() and it could be a path aliased via symbolic links, e.g., the real directory may be /home/u/repository but a symbolic link /home/u/repo may point at it, and the clone request may come as "git clone file:///home/u/repo/". A request to check if /home/u/repo is safe would be rejected if the safe.directory configuration allows /home/u/repository/ but not its alias /home/u/repo/. Normalize the path being checked before comparing with safe.directory value(s). Suggested-by: Phillip Wood Signed-off-by: Junio C Hamano --- setup.c | 16 ++++++++++---- t/t0033-safe-directory.sh | 45 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/setup.c b/setup.c index d458edcc02..45bbbe329f 100644 --- a/setup.c +++ b/setup.c @@ -1215,7 +1215,7 @@ static int canonicalize_ceiling_entry(struct string_list_item *item, } struct safe_directory_data { - const char *path; + char *path; int is_safe; }; @@ -1263,9 +1263,7 @@ static int ensure_valid_ownership(const char *gitfile, const char *worktree, const char *gitdir, struct strbuf *report) { - struct safe_directory_data data = { - .path = worktree ? worktree : gitdir - }; + struct safe_directory_data data = { 0 }; if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) && (!gitfile || is_path_owned_by_current_user(gitfile, report)) && @@ -1273,6 +1271,15 @@ static int ensure_valid_ownership(const char *gitfile, (!gitdir || is_path_owned_by_current_user(gitdir, report))) return 1; + /* + * normalize the data.path for comparison with normalized paths + * that come from the configuration file. The path is unsafe + * if it cannot be normalized. + */ + data.path = real_pathdup(worktree ? worktree : gitdir, 0); + if (!data.path) + return 0; + /* * data.path is the "path" that identifies the repository and it is * constant regardless of what failed above. data.is_safe should be @@ -1280,6 +1287,7 @@ static int ensure_valid_ownership(const char *gitfile, */ git_protected_config(safe_directory_cb, &data); + free(data.path); return data.is_safe; } diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh index 5fe61f1291..3e487e7f4b 100755 --- a/t/t0033-safe-directory.sh +++ b/t/t0033-safe-directory.sh @@ -119,4 +119,49 @@ test_expect_success 'local clone of unowned repo accepted in safe directory' ' test_path_is_dir target ' +test_expect_success SYMLINKS 'checked paths are normalized' ' + test_when_finished "rm -rf repository; rm -f repo" && + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global --unset-all safe.directory + ) && + git init repository && + ln -s repository repo && + ( + cd repository && + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + test_commit sample + ) && + + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global safe.directory "$(pwd)/repository" + ) && + git -C repository/ for-each-ref >/dev/null && + git -C repo/ for-each-ref +' + +test_expect_success SYMLINKS 'checked leading paths are normalized' ' + test_when_finished "rm -rf repository; rm -f repo" && + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global --unset-all safe.directory + ) && + mkdir -p repository && + git init repository/s && + ln -s repository repo && + ( + cd repository/s && + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + test_commit sample + ) && + + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global safe.directory "$(pwd)/repository/*" + ) && + git -C repository/s for-each-ref >/dev/null && + git -C repo/s for-each-ref +' + test_done From patchwork Sat Jul 20 22:09:13 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13737921 Received: from pb-smtp1.pobox.com (pb-smtp1.pobox.com [64.147.108.70]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DF66E1494BD for ; Sat, 20 Jul 2024 22:09:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.108.70 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721513368; cv=none; b=YdKefujgWKr+/IMNaVTg7bGpjC5Ezg/SM02Yp0Cr+lGCXy1bx+NVWuTnWiXbzc41ucmmb7/3Pkzg41s9+6720GGh1TnZv4ggzU0XgvvyV2AslAJ4lmwC5bKTZFzywcanRuE+VfYCK9ONpY1fYi8o8vBQdE4tf3KXlHC/aM9YCRg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721513368; c=relaxed/simple; bh=4cbRXQLMDVByK4C75iqJRC2Fj0Z+EikHvxM20LBwZHw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=q1l3M3Do6b1KWLMUB9/pbnpvoe4L7Cas7dJC2vBtbQqrttF8ls8z4nGr5WOpsyc+4PxPHpdRq4Xt7BPFOZIOBqGox0bvxVbJhmtGrh8Pkmub3xv7W43iQW1cpt3AVFKSYfIyCkF2tpyDYMJgt1J1uUi1QgI4kUrTEm6yVGCHUFs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=kg40n/nM; arc=none smtp.client-ip=64.147.108.70 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="kg40n/nM" Received: from pb-smtp1.pobox.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 7104B2715B; Sat, 20 Jul 2024 18:09:20 -0400 (EDT) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; s=sasl; bh=4cbRXQLMDVByK4C75iqJRC2Fj 0Z+EikHvxM20LBwZHw=; b=kg40n/nMh+MlDGebmyVbHDzYNxBGQSlB/DSRaMfu+ swkZ3ykXta+AXWzjYEn/iK0bPLUtCzlp3LhD2HElxkNE8J63L3ri46DKOA8nHv+Y WSt2dRtmo0Ztx/lXkkqN0UdN+G3NRBbkPE4Amur5m3sFApQ49gHAA0Aps7xmplSV TU= Received: from pb-smtp1.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 6918C2715A; Sat, 20 Jul 2024 18:09:20 -0400 (EDT) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.139.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp1.pobox.com (Postfix) with ESMTPSA id C418B27159; Sat, 20 Jul 2024 18:09:19 -0400 (EDT) (envelope-from gitster@pobox.com) From: Junio C Hamano To: git@vger.kernel.org Cc: Phillip Wood Subject: [PATCH 2/2] safe.directory: normalize the configured path Date: Sat, 20 Jul 2024 15:09:13 -0700 Message-ID: <20240720220915.2933266-3-gitster@pobox.com> X-Mailer: git-send-email 2.46.0-rc1-48-g0900f1888e In-Reply-To: <20240720220915.2933266-1-gitster@pobox.com> References: <20240720220915.2933266-1-gitster@pobox.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: B66338E6-46E4-11EF-9CB7-34EEED2EC81B-77302942!pb-smtp1.pobox.com The pathname of a repository comes from getcwd() and it could be a path aliased via symbolic links, e.g., the real directory may be /home/u/repository but a symbolic link /home/u/repo may point at it, and the clone request may come as "git clone file:///home/u/repo/" A request to check if /home/u/repository is safe would be rejected if the safe.directory configuration allows /home/u/repo/ but not its alias /home/u/repository/. Normalize the paths configured for the safe.directory configuration variable before comparing them with the path being checked. Suggested-by: Phillip Wood Signed-off-by: Junio C Hamano --- setup.c | 12 +++++++++++ t/t0033-safe-directory.sh | 45 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/setup.c b/setup.c index 45bbbe329f..29304d7452 100644 --- a/setup.c +++ b/setup.c @@ -1236,6 +1236,16 @@ static int safe_directory_cb(const char *key, const char *value, if (!git_config_pathname(&allowed, key, value)) { const char *check = allowed ? allowed : value; + char *to_free = real_pathdup(check, 0); + + if (!to_free) { + warning(_("safe.directory '%s' cannot be normalized"), + check); + goto next; + } else { + check = to_free; + } + if (ends_with(check, "/*")) { size_t len = strlen(check); if (!fspathncmp(check, data->path, len - 1)) @@ -1243,7 +1253,9 @@ static int safe_directory_cb(const char *key, const char *value, } else if (!fspathcmp(data->path, check)) { data->is_safe = 1; } + free(to_free); } + next: if (allowed != value) free(allowed); } diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh index 3e487e7f4b..17d45d481e 100755 --- a/t/t0033-safe-directory.sh +++ b/t/t0033-safe-directory.sh @@ -164,4 +164,49 @@ test_expect_success SYMLINKS 'checked leading paths are normalized' ' git -C repo/s for-each-ref ' +test_expect_success SYMLINKS 'configured paths are normalized' ' + test_when_finished "rm -rf repository; rm -f repo" && + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global --unset-all safe.directory + ) && + git init repository && + ln -s repository repo && + ( + cd repository && + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + test_commit sample + ) && + + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global safe.directory "$(pwd)/repo" + ) && + git -C repository/ for-each-ref >/dev/null && + git -C repo/ for-each-ref +' + +test_expect_success SYMLINKS 'configured leading paths are normalized' ' + test_when_finished "rm -rf repository; rm -f repo" && + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global --unset-all safe.directory + ) && + mkdir -p repository && + git init repository/s && + ln -s repository repo && + ( + cd repository && + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + test_commit sample + ) && + + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global safe.directory "$(pwd)/repo/*" + ) && + git -C repository/s for-each-ref >/dev/null && + git -C repo/s for-each-ref +' + test_done From patchwork Sat Jul 20 22:09:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13737922 Received: from pb-smtp21.pobox.com (pb-smtp21.pobox.com [173.228.157.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B673E143733 for ; Sat, 20 Jul 2024 22:09:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=173.228.157.53 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721513373; cv=none; b=sz7nv1xpt/TOGV1dF4Z6WbR/sAvvahTyaQ9lonovHbgX5eeIXeQleZ/I39+4jgcb0j1hrJOaM06s5wGTkQJQbmdBhiSE1Whyjak/XOPp8ehqraq/6zVfgSAGfZum1IGn/yrvmr2QNou11As2e1Cwkmqe58rNA5tWAZiy/o8VpIs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721513373; c=relaxed/simple; bh=2EnNQF34moMG0J+33113JUkH6pHfbuFLq331E6X70YA=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=pqUlENUyRVmnp+BjZav0/vQopl6b0PLNcq/o4Wf52O+7YKzjcJ9qQVSaa4zsKw+I9c65M9fL04uPxeN+XA7uPsURjRS9bJDsgXdta5BRbMIgvn8yGUMyElpEFXlrTv6m2B2Lu+fMftGjkVqS/lBCIrXRHyFpzims++ZFHGwJu4M= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=JQDF//hd; arc=none smtp.client-ip=173.228.157.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="JQDF//hd" Received: from pb-smtp21.pobox.com (unknown [127.0.0.1]) by pb-smtp21.pobox.com (Postfix) with ESMTP id 454A42B338; Sat, 20 Jul 2024 18:09:25 -0400 (EDT) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to :subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; s=sasl; bh=2EnNQF34moMG0J+33113JUkH6 pHfbuFLq331E6X70YA=; b=JQDF//hdDLRjCuKjq5ED4jnTHvq8DdREDrBoa46Tf ImVW/aCNJvZdluXGpPiAIJi8Nh+/6CDbdWPa/ALdrROyN27EKwRtxIZGBaI/8cg4 tWY519grr++tW0H5KWHL1gRryDztdyNL2Yj1UaURGbFnCksSmTC+GDlKN3Xo9iiM vw= Received: from pb-smtp21.sea.icgroup.com (unknown [127.0.0.1]) by pb-smtp21.pobox.com (Postfix) with ESMTP id 3E7282B337; Sat, 20 Jul 2024 18:09:25 -0400 (EDT) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.139.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp21.pobox.com (Postfix) with ESMTPSA id C19E62B336; Sat, 20 Jul 2024 18:09:21 -0400 (EDT) (envelope-from gitster@pobox.com) From: Junio C Hamano To: git@vger.kernel.org Subject: [PATCH 3/2] setup: use a single return path in setup_git_directory*() Date: Sat, 20 Jul 2024 15:09:14 -0700 Message-ID: <20240720220915.2933266-4-gitster@pobox.com> X-Mailer: git-send-email 2.46.0-rc1-48-g0900f1888e In-Reply-To: <20240720220915.2933266-1-gitster@pobox.com> References: <20240720220915.2933266-1-gitster@pobox.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: B791ECF8-46E4-11EF-A68E-9625FCCAB05B-77302942!pb-smtp21.pobox.com [Do not use. For illustration purposes only] Instead of sprinkling "return" all over the place, use the "assign to the result variable and then jump to the single label set up to leave the function" pattern, so that we can clean up any extra resource allocated before returning at a single place. No functional change is intended with this step, but it will be used soon. Signed-off-by: Junio C Hamano --- setup.c | 50 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/setup.c b/setup.c index 29304d7452..29e23a905c 100644 --- a/setup.c +++ b/setup.c @@ -1416,6 +1416,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, int ceil_offset = -1, min_offset = offset_1st_component(dir->buf); dev_t current_device = 0; int one_filesystem = 1; + enum discovery_result result; /* * If GIT_DIR is set explicitly, we're not going @@ -1425,7 +1426,8 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, gitdirenv = getenv(GIT_DIR_ENVIRONMENT); if (gitdirenv) { strbuf_addstr(gitdir, gitdirenv); - return GIT_DIR_EXPLICIT; + result = GIT_DIR_EXPLICIT; + goto cleanup_and_return; } if (env_ceiling_dirs) { @@ -1479,8 +1481,10 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT; gitdir_path = xstrdup(dir->buf); } - } else if (error_code != READ_GITFILE_ERR_STAT_FAILED) - return GIT_DIR_INVALID_GITFILE; + } else if (error_code != READ_GITFILE_ERR_STAT_FAILED) { + result = GIT_DIR_INVALID_GITFILE; + goto cleanup_and_return; + } } else gitfile = xstrdup(dir->buf); /* @@ -1491,16 +1495,15 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, */ strbuf_setlen(dir, offset); if (gitdirenv) { - enum discovery_result ret; const char *gitdir_candidate = gitdir_path ? gitdir_path : gitdirenv; if (ensure_valid_ownership(gitfile, dir->buf, gitdir_candidate, report)) { strbuf_addstr(gitdir, gitdirenv); - ret = GIT_DIR_DISCOVERED; + result = GIT_DIR_DISCOVERED; } else - ret = GIT_DIR_INVALID_OWNERSHIP; + result = GIT_DIR_INVALID_OWNERSHIP; /* * Earlier, during discovery, we might have allocated @@ -1514,8 +1517,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, */ free(gitdir_path); free(gitfile); - - return ret; + goto cleanup_and_return; } if (is_git_directory(dir->buf)) { @@ -1523,25 +1525,37 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, if (get_allowed_bare_repo() == ALLOWED_BARE_REPO_EXPLICIT && !is_implicit_bare_repo(dir->buf)) return GIT_DIR_DISALLOWED_BARE; - if (!ensure_valid_ownership(NULL, NULL, dir->buf, report)) - return GIT_DIR_INVALID_OWNERSHIP; - strbuf_addstr(gitdir, "."); - return GIT_DIR_BARE; + if (!ensure_valid_ownership(NULL, NULL, dir->buf, report)) { + result = GIT_DIR_INVALID_OWNERSHIP; + } else { + strbuf_addstr(gitdir, "."); + result = GIT_DIR_BARE; + } + goto cleanup_and_return; } - if (offset <= min_offset) - return GIT_DIR_HIT_CEILING; + if (offset <= min_offset) { + result = GIT_DIR_HIT_CEILING; + goto cleanup_and_return; + } while (--offset > ceil_offset && !is_dir_sep(dir->buf[offset])) ; /* continue */ - if (offset <= ceil_offset) - return GIT_DIR_HIT_CEILING; + if (offset <= ceil_offset) { + result = GIT_DIR_HIT_CEILING; + goto cleanup_and_return; + } strbuf_setlen(dir, offset > min_offset ? offset : min_offset); if (one_filesystem && - current_device != get_device_or_die(dir->buf, NULL, offset)) - return GIT_DIR_HIT_MOUNT_POINT; + current_device != get_device_or_die(dir->buf, NULL, offset)) { + result = GIT_DIR_HIT_MOUNT_POINT; + goto cleanup_and_return; + } } + +cleanup_and_return: + return result; } enum discovery_result discover_git_directory_reason(struct strbuf *commondir, From patchwork Sat Jul 20 22:09:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13737923 Received: from pb-smtp20.pobox.com (pb-smtp20.pobox.com [173.228.157.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0D58D143733 for ; Sat, 20 Jul 2024 22:09:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=173.228.157.52 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721513377; cv=none; b=Gyzf5sfF5Ej2H44O0ne0z9Fy8nEDu7yj8CXNNIXUpvDpSbRlt+GeGXftTu3G2HXflnO5pto0uQIPULxghoSycpMZn9pJxLIRn9EsKZGQUk2PGA+vUfG/jpaBLWK/MdP41VZmEUSJ/8KVDBElNPVlsqDRWuZhSkIepIxClp8Mje4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721513377; c=relaxed/simple; bh=ow1TZ4NK1632aAlwLfOaf3nyJJjw558ZRnxIHPkE9IQ=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=jF9DP5ICSAENPc5E6vwpSfsazMqn+OORmuD1j75egzaWUnXCv9Q83+tvBv3+O6KqusrLo7FJgs3ckv/SbILvWnuO/sWYmIhkNZpaWcZz6wW+Bsn28eeSlFqZnxwzw82llZyjeaYP86a+S3UaFKAjltgBEAHS88H9WESV8LCLD18= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=MuCR+cP/; arc=none smtp.client-ip=173.228.157.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="MuCR+cP/" Received: from pb-smtp20.pobox.com (unknown [127.0.0.1]) by pb-smtp20.pobox.com (Postfix) with ESMTP id 237C926908; Sat, 20 Jul 2024 18:09:29 -0400 (EDT) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to :subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; s=sasl; bh=ow1TZ4NK1632aAlwLfOaf3nyJ Jjw558ZRnxIHPkE9IQ=; b=MuCR+cP/ZF+ovoZykXfKQ6EQOFVvzNGQ5gDLDVZfP Vs5kDQrD5AsebyPYQpXA9a/QOnPy40kQa62TQh6Xd0dBTkZu2ItdEosNRNCJ4AYh 0lWQLjxcpV5JsjQprxpvf46eOrquArLb1e+e4/IDak2CN5zuyH2zCftp0AXSFDve lM= Received: from pb-smtp20.sea.icgroup.com (unknown [127.0.0.1]) by pb-smtp20.pobox.com (Postfix) with ESMTP id 1C4CE26907; Sat, 20 Jul 2024 18:09:29 -0400 (EDT) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.139.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp20.pobox.com (Postfix) with ESMTPSA id 92B2926904; Sat, 20 Jul 2024 18:09:25 -0400 (EDT) (envelope-from gitster@pobox.com) From: Junio C Hamano To: git@vger.kernel.org Subject: [PATCH 4/2] setup: cache normalized safe.directory configuration Date: Sat, 20 Jul 2024 15:09:15 -0700 Message-ID: <20240720220915.2933266-5-gitster@pobox.com> X-Mailer: git-send-email 2.46.0-rc1-48-g0900f1888e In-Reply-To: <20240720220915.2933266-1-gitster@pobox.com> References: <20240720220915.2933266-1-gitster@pobox.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: B9D79490-46E4-11EF-9128-92D9AF168FA5-77302942!pb-smtp20.pobox.com [Do not use. For illustration purposes only] The current check performed in ensure_valid_ownership() reads each safe.directory configuration item and normalizes it before checking against the path to the repository. This is OK as long as we are checking just a single directory, like in die_upon_dubious_ownership() and setup_git_directory_gently_1(). But let's pretend that the latter calls ensure_valid_ownership() many times in the loop, and demonstrate how we would avoid having to normalize the same safe.directory configuration item over and over. Signed-off-by: Junio C Hamano --- setup.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 67 insertions(+), 12 deletions(-) diff --git a/setup.c b/setup.c index 29e23a905c..75bcce0368 100644 --- a/setup.c +++ b/setup.c @@ -20,6 +20,7 @@ #include "trace2.h" #include "worktree.h" #include "exec-cmd.h" +#include "strvec.h" static int inside_git_dir = -1; static int inside_work_tree = -1; @@ -1217,6 +1218,7 @@ static int canonicalize_ceiling_entry(struct string_list_item *item, struct safe_directory_data { char *path; int is_safe; + int prenormalized; }; static int safe_directory_cb(const char *key, const char *value, @@ -1236,14 +1238,20 @@ static int safe_directory_cb(const char *key, const char *value, if (!git_config_pathname(&allowed, key, value)) { const char *check = allowed ? allowed : value; - char *to_free = real_pathdup(check, 0); - - if (!to_free) { - warning(_("safe.directory '%s' cannot be normalized"), - check); - goto next; - } else { + char *to_free = NULL; + + if (!data->prenormalized) { + to_free = real_pathdup(check, 0); + if (!to_free) { + warning(_("safe.directory '%s' " + "cannot be normalized"), + check); + goto next; + } check = to_free; + } else { + to_free = NULL; + check = value; } if (ends_with(check, "/*")) { @@ -1263,6 +1271,39 @@ static int safe_directory_cb(const char *key, const char *value, return 0; } +static int prenorm_cb(const char *key, const char *value, + const struct config_context *ctx UNUSED, void *v_) +{ + struct strvec *v = v_; + + if (strcmp(key, "safe.directory")) + return 0; + if (!value || !*value) { + strvec_clear(v); + } else if (!strcmp(value, "*")) { + strvec_push(v, value); + } else { + char *allowed = NULL; + if (!git_config_pathname(&allowed, key, value)) { + const char *ccheck = allowed ? allowed : value; + char *check = real_pathdup(ccheck, 0); + if (check) + strvec_push_nodup(v, check); + else + warning(_("safe.directory '%s' cannot be normalized"), + ccheck); + } + if (allowed != value) + free(allowed); + } + return 0; +} + +static void prenormalize_safe_directory(struct strvec *v) +{ + git_protected_config(prenorm_cb, v); +} + /* * Check if a repository is safe, by verifying the ownership of the * worktree (if any), the git directory, and the gitfile (if any). @@ -1273,6 +1314,7 @@ static int safe_directory_cb(const char *key, const char *value, */ static int ensure_valid_ownership(const char *gitfile, const char *worktree, const char *gitdir, + struct strvec *safe_cache, struct strbuf *report) { struct safe_directory_data data = { 0 }; @@ -1297,8 +1339,15 @@ static int ensure_valid_ownership(const char *gitfile, * constant regardless of what failed above. data.is_safe should be * initialized to false, and might be changed by the callback. */ - git_protected_config(safe_directory_cb, &data); - + if (!safe_cache) { + git_protected_config(safe_directory_cb, &data); + } else { + data.prenormalized = 1; + for (size_t i = 0; i < safe_cache->nr; i++) { + safe_directory_cb("safe.directory", safe_cache->v[i], + NULL, &data); + } + } free(data.path); return data.is_safe; } @@ -1309,7 +1358,7 @@ void die_upon_dubious_ownership(const char *gitfile, const char *worktree, struct strbuf report = STRBUF_INIT, quoted = STRBUF_INIT; const char *path; - if (ensure_valid_ownership(gitfile, worktree, gitdir, &report)) + if (ensure_valid_ownership(gitfile, worktree, gitdir, NULL, &report)) return; strbuf_complete(&report, '\n'); @@ -1416,6 +1465,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, int ceil_offset = -1, min_offset = offset_1st_component(dir->buf); dev_t current_device = 0; int one_filesystem = 1; + struct strvec safe_cache = STRVEC_INIT; enum discovery_result result; /* @@ -1463,6 +1513,8 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, one_filesystem = !git_env_bool("GIT_DISCOVERY_ACROSS_FILESYSTEM", 0); if (one_filesystem) current_device = get_device_or_die(dir->buf, NULL, 0); + + prenormalize_safe_directory(&safe_cache); for (;;) { int offset = dir->len, error_code = 0; char *gitdir_path = NULL; @@ -1499,7 +1551,8 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, gitdir_path ? gitdir_path : gitdirenv; if (ensure_valid_ownership(gitfile, dir->buf, - gitdir_candidate, report)) { + gitdir_candidate, + &safe_cache, report)) { strbuf_addstr(gitdir, gitdirenv); result = GIT_DIR_DISCOVERED; } else @@ -1525,7 +1578,8 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, if (get_allowed_bare_repo() == ALLOWED_BARE_REPO_EXPLICIT && !is_implicit_bare_repo(dir->buf)) return GIT_DIR_DISALLOWED_BARE; - if (!ensure_valid_ownership(NULL, NULL, dir->buf, report)) { + if (!ensure_valid_ownership(NULL, NULL, dir->buf, + &safe_cache, report)) { result = GIT_DIR_INVALID_OWNERSHIP; } else { strbuf_addstr(gitdir, "."); @@ -1555,6 +1609,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, } cleanup_and_return: + strvec_clear(&safe_cache); return result; }