Skip to content

Commit

Permalink
Connector warns on concurrent PC/SC calls
Browse files Browse the repository at this point in the history
Add warning logs into Smart Card Connector whenever a client
application emits illegal concurrent PC/SC requests: an
SCARDCONTEXT must only be used from a single thread (with the
only exception of SCardCancel that can be called concurrently).

This is preparation for disallowing concurrent PC/SC calls in the
future as planned in #136. Concurrent calls are not allowed by the
PC/SC API specification, and the PC/SC-Lite's implementation
breaks in subtle ways when this is violated.
  • Loading branch information
emaxx-google committed Dec 8, 2022
1 parent a1c7a76 commit d2a126a
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
#include <cstddef>
#include <cstring>
#include <memory>
#include <mutex>
#include <sstream>
#include <string>
#include <thread>
#include <tuple>
#include <vector>
Expand Down Expand Up @@ -130,6 +133,14 @@ void CleanupHandles(const std::string& logging_prefix,
CloseLeftHandles(logging_prefix, s_card_contexts);
}

// Returns whether the PC/SC function can be called concurrently with other
// functions that operate on the same `SCARDCONTEXT`.
bool IsConcurrentCallAllowed(const std::string& function_name) {
// Per the PC/SC API specification, only cancellation can be requested from a
// different thread.
return function_name == "SCardCancel";
}

} // namespace

PcscLiteClientRequestProcessor::PcscLiteClientRequestProcessor(
Expand Down Expand Up @@ -202,6 +213,8 @@ void PcscLiteClientRequestProcessor::ProcessRequest(
<< "\"";
}

LeaveConcurrencyCheckScope(request.function_name);

result_callback(std::move(result));
}

Expand All @@ -210,6 +223,12 @@ void PcscLiteClientRequestProcessor::AsyncProcessRequest(
std::shared_ptr<PcscLiteClientRequestProcessor> request_processor,
RemoteCallRequestPayload request,
RequestReceiver::ResultCallback result_callback) {
// This is paired with the call to `LeaveConcurrencyCheckScope()` in
// `ProcessRequest()`. We enter the scope here in order to be able to catch
// concurrency violations even when they occur so fast that the thread created
// below didn't have chance to start and do any work.
request_processor->EnterConcurrencyCheckScope(request.function_name);

std::thread(&PcscLiteClientRequestProcessor::ProcessRequest,
request_processor, std::move(request), result_callback)
.detach();
Expand Down Expand Up @@ -1117,4 +1136,42 @@ void PcscLiteClientRequestProcessor::OnSCardHandleRevoked(
s_card_handles_registry_.RemoveHandle(s_card_handle);
}

void PcscLiteClientRequestProcessor::EnterConcurrencyCheckScope(
const std::string& starting_function_name) {
if (IsConcurrentCallAllowed(starting_function_name)) {
return;
}

std::vector<std::string> concurrent_functions;
{
std::unique_lock<std::mutex> lock(mutex_);
concurrent_functions.assign(currently_running_functions_.begin(),
currently_running_functions_.end());
currently_running_functions_.insert(starting_function_name);
}

if (concurrent_functions.empty()) {
return;
}

std::ostringstream str;
for (const auto& name : concurrent_functions) {
str << ", " << name;
}
GOOGLE_SMART_CARD_LOG_WARNING
<< logging_prefix_ << "Client violates threading: concurrent calls of "
<< starting_function_name << str.str()
<< ". Future releases of Smart Card Connector will forbid this.";
}

void PcscLiteClientRequestProcessor::LeaveConcurrencyCheckScope(
const std::string& finished_function_name) {
if (IsConcurrentCallAllowed(finished_function_name)) {
return;
}
std::unique_lock<std::mutex> lock(mutex_);
currently_running_functions_.erase(
currently_running_functions_.find(finished_function_name));
}

} // namespace google_smart_card
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#include <cstddef>
#include <functional>
#include <memory>
#include <mutex>
#include <set>
#include <string>
#include <unordered_map>
#include <vector>
Expand Down Expand Up @@ -228,6 +230,9 @@ class PcscLiteClientRequestProcessor final
void OnSCardContextRevoked(SCARDCONTEXT s_card_context);
void OnSCardHandleRevoked(SCARDHANDLE s_card_handle);

void EnterConcurrencyCheckScope(const std::string& starting_function_name);
void LeaveConcurrencyCheckScope(const std::string& finished_function_name);

const int64_t client_handler_id_;
const std::string client_name_for_log_;
const LogSeverity status_log_severity_;
Expand All @@ -238,6 +243,9 @@ class PcscLiteClientRequestProcessor final
// used to implement the client isolation: one client shouldn't be able to use
// contexts/handles belonging to the other one.
PcscLiteClientHandlesRegistry s_card_handles_registry_;

mutable std::mutex mutex_;
std::multiset<std::string> currently_running_functions_;
};

} // namespace google_smart_card
Expand Down

0 comments on commit d2a126a

Please sign in to comment.