diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index b2ce7d29b94..10891c861a3 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -94,7 +94,7 @@ pub fn main(gctx: &mut GlobalContext) -> CliResult { global_args, Some(&exec), )?; - super::init_git(gctx); + super::init_git(); exec.exec(gctx, subcommand_args)?; } diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index 41252a61296..b12d5f0a44f 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -1,6 +1,4 @@ use cargo::core::features; -use cargo::util::network::http::http_handle; -use cargo::util::network::http::needs_custom_http_transport; use cargo::util::{self, CargoResult, closest_msg, command_prelude}; use cargo_util::{ProcessBuilder, ProcessError}; use cargo_util_schemas::manifest::StringOrVec; @@ -370,7 +368,7 @@ fn search_directories(gctx: &GlobalContext) -> Vec { /// Initialize libgit2. #[tracing::instrument(skip_all)] -fn init_git(gctx: &GlobalContext) { +fn init_git() { // Disabling the owner validation in git can, in theory, lead to code execution // vulnerabilities. However, libgit2 does not launch executables, which is the foundation of // the original security issue. Meanwhile, issues with refusing to load git repos in @@ -392,38 +390,4 @@ fn init_git(gctx: &GlobalContext) { git2::opts::set_verify_owner_validation(false) .expect("set_verify_owner_validation should never fail"); } - - init_git_transports(gctx); -} - -/// Configure libgit2 to use libcurl if necessary. -/// -/// If the user has a non-default network configuration, then libgit2 will be -/// configured to use libcurl instead of the built-in networking support so -/// that those configuration settings can be used. -#[tracing::instrument(skip_all)] -fn init_git_transports(gctx: &GlobalContext) { - match needs_custom_http_transport(gctx) { - Ok(true) => {} - _ => return, - } - - let handle = match http_handle(gctx) { - Ok(handle) => handle, - Err(..) => return, - }; - - // The unsafety of the registration function derives from two aspects: - // - // 1. This call must be synchronized with all other registration calls as - // well as construction of new transports. - // 2. The argument is leaked. - // - // We're clear on point (1) because this is only called at the start of this - // binary (we know what the state of the world looks like) and we're mostly - // clear on point (2) because we'd only free it after everything is done - // anyway - unsafe { - git2_curl::register(handle); - } } diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index f181072858b..c4124aaa6d9 100644 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -7,10 +7,16 @@ //! - : targeted at cargo contributors //! - Updated on each update of the `cargo` submodule in `rust-lang/rust` //! -//! > This library is maintained by the Cargo team, primarily for use by Cargo -//! > and not intended for external use (except as a transitive dependency). This -//! > crate may make major changes to its APIs. See [The Cargo Book: -//! > External tools] for more on this topic. +//!
+//! +//! This library is maintained by the Cargo team, primarily for use by Cargo +//! and not intended for external use. +//! This crate may make major changes to its APIs or assumptions about how it is invoked (e.g. +//! when it is safe to call [git2::transport::register]). +//! +//! See [The Cargo Book: External tools] for alternatives to using this library. +//! +//!
//! //! ## Overview //! diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index 8b31e73da7b..6b5947e5e64 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -9,6 +9,8 @@ use crate::sources::git::source::GitSource; use crate::sources::source::Source as _; use crate::util::HumanBytes; use crate::util::errors::{CargoResult, GitCliError}; +use crate::util::network::http::http_handle; +use crate::util::network::http::needs_custom_http_transport; use crate::util::{GlobalContext, IntoUrl, MetricsCounter, Progress, network}; use anyhow::{Context as _, anyhow}; @@ -23,6 +25,7 @@ use std::borrow::Cow; use std::path::{Path, PathBuf}; use std::process::Command; use std::str; +use std::sync::Once; use std::sync::atomic::{AtomicBool, Ordering}; use std::time::{Duration, Instant}; @@ -1308,6 +1311,24 @@ fn fetch_with_libgit2( ) -> CargoResult<()> { debug!(target: "git-fetch", backend = "libgit2"); + static INIT: Once = Once::new(); + INIT.call_once(|| { + // SAFETY: + // + // The unsafety of the registration function derives from two aspects: + // + // 1. This call must be synchronized with all other registration calls as + // well as construction of new transports. + // 2. The argument is leaked. + // + // We're clear on point (1) because this is the only `register` call we make in the + // cargo-binary and cargo-library is not officially supported and it is under a lock. + // Technically, `git2_curl` also has a lock but that isn't part of their API guarantees. + // + // We're mostly clear on point (2) because we'd only free it after everything is done anyway + unsafe { init_git_transports(gctx) }; + }); + let git_config = git2::Config::open_default()?; with_fetch_options(&git_config, remote_url, gctx, &mut |mut opts| { if tags { @@ -1356,6 +1377,32 @@ fn fetch_with_libgit2( }) } +/// Configure libgit2 to use libcurl if necessary. +/// +/// If the user has a non-default network configuration, then libgit2 will be +/// configured to use libcurl instead of the built-in networking support so +/// that those configuration settings can be used. +/// +/// # Safety +/// +/// See [git2_curl::register] +#[tracing::instrument(skip_all)] +unsafe fn init_git_transports(gctx: &GlobalContext) { + match needs_custom_http_transport(gctx) { + Ok(true) => {} + _ => return, + } + + let handle = match http_handle(gctx) { + Ok(handle) => handle, + Err(..) => return, + }; + + unsafe { + git2_curl::register(handle); + } +} + /// Attempts to `git gc` a repository. /// /// Cargo has a bunch of long-lived git repositories in its global cache and