diff --git a/src/pam/main.cc b/src/pam/main.cc index 5dc5b61..d435f3e 100644 --- a/src/pam/main.cc +++ b/src/pam/main.cc @@ -49,7 +49,7 @@ const auto DEFAULT_TIMEOUT = std::chrono::duration(2500); -#define S(msg) gettext (msg) +#define S(msg) gettext(msg) /** * Inspect the status code returned by the compare process @@ -112,7 +112,7 @@ auto howdy_error(int status, * @param conv_function PAM conversation function * @return Returns the conversation function return code */ - auto howdy_msg(char *username, int status, const INIReader &reader, +auto howdy_msg(char *username, int status, const INIReader &reader, const std::function &conv_function) -> int { if (status != EXIT_SUCCESS) { @@ -277,7 +277,7 @@ auto identify(pam_handle_t *pamh, int flags, int argc, const char **argv, // This task wait for the status of the python subprocess (we don't want a // zombie process) - optional_task child_task(std::packaged_task([&] { + optional_task child_task([&] { int status; wait(&status); @@ -292,29 +292,26 @@ auto identify(pam_handle_t *pamh, int flags, int argc, const char **argv, cv.notify_one(); return status; - })); + }); child_task.activate(); // This task waits for the password input (if the workaround wants it) - optional_task> pass_task( - std::packaged_task()>([&] { - char *auth_tok_ptr = nullptr; - int pam_res = - pam_get_authtok(pamh, PAM_AUTHTOK, - const_cast(&auth_tok_ptr), nullptr); - { - std::unique_lock lk(m); - ConfirmationType type = - confirmation_type.load(std::memory_order_relaxed); - if (type == ConfirmationType::Unset) { - confirmation_type.store(ConfirmationType::Pam, - std::memory_order_relaxed); - } - } - cv.notify_one(); + optional_task> pass_task([&] { + char *auth_tok_ptr = nullptr; + int pam_res = pam_get_authtok( + pamh, PAM_AUTHTOK, const_cast(&auth_tok_ptr), nullptr); + { + std::unique_lock lk(m); + ConfirmationType type = confirmation_type.load(std::memory_order_relaxed); + if (type == ConfirmationType::Unset) { + confirmation_type.store(ConfirmationType::Pam, + std::memory_order_relaxed); + } + } + cv.notify_one(); - return std::tuple(pam_res, auth_tok_ptr); - })); + return std::tuple(pam_res, auth_tok_ptr); + }); if (auth_tok) { pass_task.activate(); @@ -331,8 +328,8 @@ auto identify(pam_handle_t *pamh, int flags, int argc, const char **argv, // If the workaround is native if (auth_tok) { - // We cancel the thread using pthread, pam_get_authtok seems to be a - // cancellation point + // UNSAFE: We cancel the thread using pthread, pam_get_authtok seems to be + // a cancellation point if (pass_task.is_active()) { pass_task.stop(true); } diff --git a/src/pam/optional_task.hh b/src/pam/optional_task.hh index 892836c..f1fc9db 100644 --- a/src/pam/optional_task.hh +++ b/src/pam/optional_task.hh @@ -6,18 +6,19 @@ #include #include +// A task executed only if activated. template class optional_task { std::thread _thread; std::packaged_task _task; std::future _future; - std::atomic _spawned; - std::atomic _is_active; + bool _spawned; + bool _is_active; public: - explicit optional_task(std::packaged_task task); + explicit optional_task(std::function fn); void activate(); - template - auto wait(std::chrono::duration dur) -> std::future_status; + template + auto wait(std::chrono::duration dur) -> std::future_status; auto get() -> T; auto is_active() -> bool; void stop(bool force); @@ -25,22 +26,28 @@ public: }; template -optional_task::optional_task(std::packaged_task t) - : _task(std::move(t)), _future(_task.get_future()) {} +optional_task::optional_task(std::function fn) + : _task(std::packaged_task(std::move(fn))), + _future(_task.get_future()) {} +// Create a new thread and launch the task on it. template void optional_task::activate() { _thread = std::thread(std::move(_task)); _spawned = true; _is_active = true; } +// Wait for `dur` time and return a `future` status. template -template -auto optional_task::wait(std::chrono::duration dur) +template +auto optional_task::wait(std::chrono::duration dur) -> std::future_status { return _future.wait_for(dur); } +// Get the value. +// WARNING: The function hould be run only if the task has successfully been +// stopped. template auto optional_task::get() -> T { assert(!_is_active && _spawned); return _future.get(); @@ -50,6 +57,11 @@ template auto optional_task::is_active() -> bool { return _is_active; } +// Stop the thread: +// - if `force` is `false`, by joining the thread. +// - if `force` is `true`, by cancelling the thread using `pthread_cancel`. +// WARNING: This function should be used with extreme caution when `force` is +// set to `true`. template void optional_task::stop(bool force) { if (!(_is_active && _thread.joinable()) && _spawned) { _is_active = false;