Skip to content

Commit c663f7e

Browse files
committed
libstore: Fix reentrancy in AwsCredentialProviderImpl::getCredentialsRaw
Old code would do very much incorrect reentrancy crimes (trying to do an erase inside the emplace callback). This would fail miserably with an assertion in Boost: terminating due to unexpected unrecoverable internal error: Assertion '(!find(px))&&("reentrancy not allowed")' failed in boost::unordered::detail::foa::entry_trace::entry_trace(const void *) at include/boost/unordered/detail/foa/reentrancy_check.hpp:33 This is trivially reproduced by using any S3 URL with a non-empty profile: nix-prefetch-url "s3://happy/crash?profile=default"
1 parent 3c03050 commit c663f7e

File tree

1 file changed

+37
-47
lines changed

1 file changed

+37
-47
lines changed

src/libstore/aws-creds.cc

Lines changed: 37 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -96,67 +96,57 @@ class AwsCredentialProviderImpl : public AwsCredentialProvider
9696
}
9797
}
9898

99+
std::shared_ptr<Aws::Crt::Auth::ICredentialsProvider> createProviderForProfile(const std::string & profile);
100+
99101
private:
100102
Aws::Crt::ApiHandle apiHandle;
101103
boost::concurrent_flat_map<std::string, std::shared_ptr<Aws::Crt::Auth::ICredentialsProvider>>
102104
credentialProviderCache;
103105
};
104106

107+
std::shared_ptr<Aws::Crt::Auth::ICredentialsProvider>
108+
AwsCredentialProviderImpl::createProviderForProfile(const std::string & profile)
109+
{
110+
debug(
111+
"[pid=%d] creating new AWS credential provider for profile '%s'",
112+
getpid(),
113+
profile.empty() ? "(default)" : profile.c_str());
114+
115+
if (profile.empty()) {
116+
Aws::Crt::Auth::CredentialsProviderChainDefaultConfig config;
117+
config.Bootstrap = Aws::Crt::ApiHandle::GetOrCreateStaticDefaultClientBootstrap();
118+
return Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderChainDefault(config);
119+
}
120+
121+
Aws::Crt::Auth::CredentialsProviderProfileConfig config;
122+
config.Bootstrap = Aws::Crt::ApiHandle::GetOrCreateStaticDefaultClientBootstrap();
123+
// This is safe because the underlying C library will copy this string
124+
// c.f. https://github.com/awslabs/aws-c-auth/blob/main/source/credentials_provider_profile.c#L220
125+
config.ProfileNameOverride = Aws::Crt::ByteCursorFromCString(profile.c_str());
126+
return Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderProfile(config);
127+
}
128+
105129
AwsCredentials AwsCredentialProviderImpl::getCredentialsRaw(const std::string & profile)
106130
{
107-
// Get or create credential provider with caching
108131
std::shared_ptr<Aws::Crt::Auth::ICredentialsProvider> provider;
109132

110-
// Use try_emplace_and_cvisit for atomic get-or-create
111-
// This prevents race conditions where multiple threads create providers
112133
credentialProviderCache.try_emplace_and_cvisit(
113134
profile,
114-
nullptr, // Placeholder - will be replaced in f1 before any thread can see it
115-
[&](auto & kv) {
116-
// f1: Called atomically during insertion with non-const reference
117-
// Other threads are blocked until we finish, so nullptr is never visible
118-
debug(
119-
"[pid=%d] creating new AWS credential provider for profile '%s'",
120-
getpid(),
121-
profile.empty() ? "(default)" : profile.c_str());
122-
123-
try {
124-
if (profile.empty()) {
125-
Aws::Crt::Auth::CredentialsProviderChainDefaultConfig config;
126-
config.Bootstrap = Aws::Crt::ApiHandle::GetOrCreateStaticDefaultClientBootstrap();
127-
kv.second = Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderChainDefault(config);
128-
} else {
129-
Aws::Crt::Auth::CredentialsProviderProfileConfig config;
130-
config.Bootstrap = Aws::Crt::ApiHandle::GetOrCreateStaticDefaultClientBootstrap();
131-
// This is safe because the underlying C library will copy this string
132-
// c.f. https://github.com/awslabs/aws-c-auth/blob/main/source/credentials_provider_profile.c#L220
133-
config.ProfileNameOverride = Aws::Crt::ByteCursorFromCString(profile.c_str());
134-
kv.second = Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderProfile(config);
135-
}
136-
137-
if (!kv.second) {
138-
throw AwsAuthError(
139-
"Failed to create AWS credentials provider for %s",
140-
profile.empty() ? "default profile" : fmt("profile '%s'", profile));
141-
}
142-
143-
provider = kv.second;
144-
} catch (Error & e) {
145-
// Exception during creation - remove the entry to allow retry
146-
credentialProviderCache.erase(profile);
147-
e.addTrace({}, "for AWS profile: %s", profile.empty() ? "(default)" : profile);
148-
throw;
149-
} catch (...) {
150-
// Non-Error exception - still need to clean up
151-
credentialProviderCache.erase(profile);
152-
throw;
153-
}
154-
},
155-
[&](const auto & kv) {
156-
// f2: Called if key already exists (const reference)
157-
provider = kv.second;
135+
nullptr,
136+
[&](auto & kv) { provider = kv.second = createProviderForProfile(profile); },
137+
[&](const auto & kv) { provider = kv.second; });
138+
139+
if (!provider) {
140+
credentialProviderCache.erase_if(profile, [](const auto & kv) {
141+
[[maybe_unused]] auto [_, provider] = kv;
142+
return !provider;
158143
});
159144

145+
throw AwsAuthError(
146+
"Failed to create AWS credentials provider for %s",
147+
profile.empty() ? "default profile" : fmt("profile '%s'", profile));
148+
}
149+
160150
return getCredentialsFromProvider(provider);
161151
}
162152

0 commit comments

Comments
 (0)