Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unix drive-by tweaks #3847

Open
wants to merge 17 commits into
base: update-libssh
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion 3rd-party/libssh/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ file(STRINGS libssh/CMakeLists.txt libssh_VERSION REGEX "^project\\(libssh")
file(STRINGS libssh/CMakeLists.txt LIBRARY_VERSION REGEX "^set\\(LIBRARY_VERSION")
file(STRINGS libssh/CMakeLists.txt LIBRARY_SOVERSION REGEX "^set\\(LIBRARY_SOVERSION")

string(REGEX MATCH "^project\\(libssh VERSION ([0-9]+)\\.([0-9]+)\\.([0-9]+) LANGUAGES C\\)$"
string(REGEX MATCH "^project\\(libssh VERSION ([0-9]+)\\.([0-9]+)\\.([0-9]+) LANGUAGES C CXX\\)$"
libssh_VERSION "${libssh_VERSION}")
if (NOT libssh_VERSION)
message(FATAL_ERROR "unable to find libssh project version")
Expand Down
2 changes: 1 addition & 1 deletion 3rd-party/libssh/libssh
2 changes: 1 addition & 1 deletion 3rd-party/submodule_info.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Version: 1.52.1 (+[our patches](https://github.com/CanonicalLtd/grpc/compare/v1.
<https://github.com/grpc/grpc/releases>

### libssh
Version: 0.10.6 (+[our patches](https://github.com/canonical/libssh/compare/libssh-0.10.6...multipass)) |
Version: 0.11.1 (+[our patches](https://github.com/canonical/libssh/compare/libssh-0.11.1...multipass)) |
<https://github.com/canonical/libssh.git> |
<https://www.libssh.org/>

Expand Down
6 changes: 5 additions & 1 deletion include/multipass/platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ std::unique_ptr<Process> make_sshfs_server_process(const SSHFSServerConfig& conf
std::unique_ptr<Process> make_process(std::unique_ptr<ProcessSpec>&& process_spec);
int symlink_attr_from(const char* path, sftp_attributes_struct* attr);

std::function<int()> make_quit_watchdog(); // call while single-threaded; call result later, in dedicated thread
// Creates a function that will wait for signals or until the passed function returns false.
// The passed function is checked every `period` milliseconds.
// If a signal is received the optional contains it, otherwise the optional is empty.
std::function<std::optional<int>(const std::function<bool()>&)> make_quit_watchdog(
const std::chrono::milliseconds& period); // call while single-threaded; call result later, in dedicated thread

std::string reinterpret_interface_id(const std::string& ux_id); // give platforms a chance to reinterpret network IDs

Expand Down
3 changes: 1 addition & 2 deletions include/multipass/platform_unix.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@
#ifndef MULTIPASS_PLATFORM_UNIX_H
#define MULTIPASS_PLATFORM_UNIX_H

#include <csignal>
#include <vector>

#include <signal.h>

namespace multipass
{
namespace platform
Expand Down
6 changes: 1 addition & 5 deletions include/multipass/ssh/ssh_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,7 @@ class SSHKeyProvider;
class SSHSession
{
public:
SSHSession(const std::string& host,
int port,
const std::string& ssh_username,
const SSHKeyProvider& key_provider,
const std::chrono::milliseconds timeout = std::chrono::seconds(20));
SSHSession(const std::string& host, int port, const std::string& ssh_username, const SSHKeyProvider& key_provider);

// just being explicit (unique_ptr member already caused these to be deleted)
SSHSession(const SSHSession&) = delete;
Expand Down
5 changes: 3 additions & 2 deletions src/platform/console/unix_console.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,14 @@ mp::UnixConsole::UnixConsole(ssh_channel channel, UnixTerminal* term) : term{ter

if (term->is_live())
{
setup_console();

const char* term_type = std::getenv("TERM");
term_type = (term_type == nullptr) ? "xterm" : term_type;

update_local_pty_size(term->cout_fd());
ssh_channel_request_pty_size(channel, term_type, local_pty_size.columns, local_pty_size.rows);

// set stdin to Raw Mode after libssh inherits sane settings from stdin.
setup_console();
}
}

Expand Down
53 changes: 46 additions & 7 deletions src/platform/platform_unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include <multipass/auto_join_thread.h>
#include <multipass/format.h>
#include <multipass/platform.h>
#include <multipass/platform_unix.h>
#include <multipass/utils.h>

#include <grp.h>
#include <scope_guard.hpp>
#include <sys/stat.h>
#include <sys/time.h>
#include <unistd.h>
Expand Down Expand Up @@ -170,16 +172,53 @@

sigset_t mp::platform::make_and_block_signals(const std::vector<int>& sigs)
{
auto sigset{make_sigset(sigs)};
pthread_sigmask(SIG_BLOCK, &sigset, nullptr);
const auto sigset{make_sigset(sigs)};
if (const auto ec = pthread_sigmask(SIG_BLOCK, &sigset, nullptr); ec)
throw std::runtime_error(fmt::format("Failed to block signals: {}", strerror(ec)));

Check warning on line 177 in src/platform/platform_unix.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/platform_unix.cpp#L175-L177

Added lines #L175 - L177 were not covered by tests

return sigset;
}

std::function<int()> mp::platform::make_quit_watchdog()
template <class Rep, class Period>
timespec make_timespec(std::chrono::duration<Rep, Period> duration)
{
const auto seconds = std::chrono::duration_cast<std::chrono::seconds>(duration);
return timespec{seconds.count(), std::chrono::duration_cast<std::chrono::nanoseconds>(duration - seconds).count()};
}

std::function<std::optional<int>(const std::function<bool()>&)> mp::platform::make_quit_watchdog(
const std::chrono::milliseconds& period)
{
return [sigset = make_and_block_signals({SIGQUIT, SIGTERM, SIGHUP})]() {
int sig = -1;
sigwait(&sigset, &sig);
return sig;
return [sigset = make_and_block_signals({SIGQUIT, SIGTERM, SIGHUP, SIGUSR2}),
period](const std::function<bool()>& condition) -> std::optional<int> {
std::mutex sig_mtx;
std::condition_variable sig_cv;
int sig = SIGUSR2;

// A signal generator that triggers after `timeout`
AutoJoinThread signaler([&sig_mtx, &sig_cv, &sig, &period, signalee = pthread_self()] {
std::unique_lock lock(sig_mtx);
while (!sig_cv.wait_for(lock, period, [&sig] { return sig != SIGUSR2; }))
{
pthread_kill(signalee, SIGUSR2);
}
});

// wait on signals and condition
int ret = SIGUSR2;
while (ret == SIGUSR2 && condition())
{
// can't use sigtimedwait since macOS doesn't support it
sigwait(&sigset, &ret);
}

{
std::lock_guard lock(sig_mtx);
sig = ret == SIGUSR2 ? 0 : ret;
}
sig_cv.notify_all();

// if sig is SIGUSR2 then we know we're exiting because condition() was false
return ret == SIGUSR2 ? std::nullopt : std::make_optional(sig);
};
}
5 changes: 2 additions & 3 deletions src/ssh/ssh_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,13 @@ namespace mpl = multipass::logging;
mp::SSHSession::SSHSession(const std::string& host,
int port,
const std::string& username,
const SSHKeyProvider& key_provider,
const std::chrono::milliseconds timeout)
const SSHKeyProvider& key_provider)
: session{ssh_new(), ssh_free}, mut{}
{
if (session == nullptr)
throw mp::SSHException("could not allocate ssh session");

const long timeout_secs = std::chrono::duration_cast<std::chrono::seconds>(timeout).count();
const long timeout_secs = std::numeric_limits<long>::max();
const int nodelay{1};
auto ssh_dir = QDir(MP_STDPATHS.writableLocation(StandardPaths::AppConfigLocation)).filePath("ssh").toStdString();

Expand Down
19 changes: 16 additions & 3 deletions src/sshfs_mount/sshfs_mount.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,21 +151,29 @@ auto make_sftp_server(mp::SSHSession&& session, const std::string& source, const

} // namespace

mp::SshfsMount::SshfsMount(SSHSession&& session, const std::string& source, const std::string& target,
const mp::id_mappings& gid_mappings, const mp::id_mappings& uid_mappings)
mp::SshfsMount::SshfsMount(SSHSession&& session,
const std::string& source,
const std::string& target,
const mp::id_mappings& gid_mappings,
const mp::id_mappings& uid_mappings)
: sftp_server{make_sftp_server(std::move(session), source, target, gid_mappings, uid_mappings)},
sftp_thread{[this]() {
sftp_thread{[this] {
state.store(State::Running, std::memory_order_release);

mp::top_catch_all(category, [this] {
std::cout << "Connected" << std::endl;
sftp_server->run();
std::cout << "Stopped" << std::endl;
});

state.store(State::Stopped, std::memory_order_release);
}}
{
}

mp::SshfsMount::~SshfsMount()
{
state.store(State::Stopped, std::memory_order_release);
stop();
}

Expand All @@ -175,3 +183,8 @@ void mp::SshfsMount::stop()
if (sftp_thread.joinable())
sftp_thread.join();
}

bool mp::SshfsMount::alive() const
{
return state.load(std::memory_order_acquire) != State::Stopped;
}
10 changes: 10 additions & 0 deletions src/sshfs_mount/sshfs_mount.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,17 @@ class SshfsMount

void stop();

[[nodiscard]] bool alive() const;

private:
enum class State
{
Unstarted,
Running,
Stopped
};

std::atomic<State> state{State::Unstarted};
// sftp_server Doesn't need to be a pointer, but done for now to avoid bringing sftp.h
// which has an error with -pedantic.
std::unique_ptr<SftpServer> sftp_server;
Expand Down
13 changes: 9 additions & 4 deletions src/sshfs_mount/sshfs_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,22 @@ int main(int argc, char* argv[])

try
{
auto watchdog = mpp::make_quit_watchdog(); // called while there is only one thread
auto watchdog =
mpp::make_quit_watchdog(std::chrono::milliseconds{500}); // called while there is only one thread

mp::SSHSession session{host, port, username, mp::SSHClientKeyProvider{priv_key_blob}};
mp::SshfsMount sshfs_mount(std::move(session), source_path, target_path, gid_mappings, uid_mappings);

// ssh lives on its own thread, use this thread to listen for quit signal
if (int sig = watchdog())
cout << "Received signal " << sig << ". Stopping" << endl;
auto sig = watchdog([&sshfs_mount] { return sshfs_mount.alive(); });

if (sig.has_value())
cout << "Received signal " << *sig << ". Stopping" << endl;
else
cerr << "SFTP server thread stopped unexpectedly." << endl;

sshfs_mount.stop();
exit(0);
exit(sig.has_value() ? EXIT_SUCCESS : EXIT_FAILURE);
}
catch (const mp::SSHFSMissingError&)
{
Expand Down
Loading