Commit 5907b6cbb5c724f9729da59a644271b4258d122e started to handle Lock/Unlock events from Session at VDAgent. That seemed to work fine but as pointed by Andrei at [0], it does not cover the following situation: > It fails for next test-case: > > * Connect with RV to VM > * Lock VM (ctrl-alt-del) > * Close RV > * Connect with RV to the VM again > * Do not unlock session. DnD files from client to locked VM. > > Result : Files are copied to VM. [0] https://bugzilla.redhat.com/show_bug.cgi?id=1323628#c6 There is also race between receiving sessions events (like WTS_SESSION_LOCK/UNLOCK) and the CONTROL_DESKTOP_SWITCH control action, which eventually calls WTSUnRegisterSessionNotification(). To solve this situation, we need VDService to inform VDAgent if Session is Locked or not, always. - We have two new events to trigger both lock/unlock events between VDService and VDAgent. - VDAgent is always initialized with session_is_locked = true Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1323628 Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx> Reported-by: Andrei Stepanov <astepano@xxxxxxxxxx> --- common/vdcommon.h | 2 ++ vdagent/vdagent.cpp | 34 ++++++++++++++++++++++++++++------ vdservice/vdservice.cpp | 20 ++++++++++++++++++++ 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/common/vdcommon.h b/common/vdcommon.h index c1920e9..6b2b5f1 100644 --- a/common/vdcommon.h +++ b/common/vdcommon.h @@ -66,6 +66,8 @@ typedef Mutex mutex_t; #define VD_AGENT_REGISTRY_KEY "SOFTWARE\\Red Hat\\Spice\\vdagent\\" #define VD_AGENT_STOP_EVENT TEXT("Global\\vdagent_stop_event") +#define VD_AGENT_SESSION_LOCKED_EVENT TEXT("Global\\vdagent_session_locked_event") +#define VD_AGENT_SESSION_UNLOCKED_EVENT TEXT("Global\\vdagent_session_unlocked_event") #if defined __GNUC__ #define ALIGN_GCC __attribute__ ((packed)) diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp index e3ba14b..95f7eef 100644 --- a/vdagent/vdagent.cpp +++ b/vdagent/vdagent.cpp @@ -148,6 +148,8 @@ private: DWORD _input_time; HANDLE _control_event; HANDLE _stop_event; + HANDLE _session_locked_event; + HANDLE _session_unlocked_event; VDAgentMessage* _in_msg; uint32_t _in_msg_pos; bool _pending_input; @@ -207,11 +209,13 @@ VDAgent::VDAgent() , _input_time (0) , _control_event (NULL) , _stop_event (NULL) + , _session_locked_event (NULL) + , _session_unlocked_event (NULL) , _in_msg (NULL) , _in_msg_pos (0) , _pending_input (false) , _running (false) - , _session_is_locked (false) + , _session_is_locked (true) , _desktop_switch (false) , _desktop_layout (NULL) , _display_setting (VD_AGENT_REGISTRY_KEY) @@ -309,6 +313,8 @@ bool VDAgent::run() return false; } _stop_event = OpenEvent(SYNCHRONIZE, FALSE, VD_AGENT_STOP_EVENT); + _session_locked_event = OpenEvent(SYNCHRONIZE, FALSE, VD_AGENT_SESSION_LOCKED_EVENT); + _session_unlocked_event = OpenEvent(SYNCHRONIZE, FALSE, VD_AGENT_SESSION_UNLOCKED_EVENT); memset(&wcls, 0, sizeof(wcls)); wcls.lpfnWndProc = &VDAgent::wnd_proc; wcls.lpszClassName = VD_AGENT_WINCLASS_NAME; @@ -479,13 +485,15 @@ void VDAgent::input_desktop_message_loop() void VDAgent::event_dispatcher(DWORD timeout, DWORD wake_mask) { - HANDLE events[2]; + HANDLE events[4]; DWORD event_count = 1; DWORD wait_ret; MSG msg; enum { CONTROL_ACTION, STOP_ACTION, + SESSION_LOCKED_ACTION, + SESSION_UNLOCKED_ACTION, } actions[SPICE_N_ELEMENTS(events)], action; events[0] = _control_event; @@ -496,6 +504,16 @@ void VDAgent::event_dispatcher(DWORD timeout, DWORD wake_mask) event_count++; } + if (_session_unlocked_event && _session_locked_event) { + events[event_count] = _session_locked_event; + actions[event_count] = SESSION_LOCKED_ACTION; + event_count++; + + events[event_count] = _session_unlocked_event; + actions[event_count] = SESSION_UNLOCKED_ACTION; + event_count++; + } + wait_ret = MsgWaitForMultipleObjectsEx(event_count, events, timeout, wake_mask, MWMO_ALERTABLE); if (wait_ret == WAIT_OBJECT_0 + event_count) { while (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) { @@ -520,6 +538,14 @@ void VDAgent::event_dispatcher(DWORD timeout, DWORD wake_mask) vd_printf("%s: received stop event", __func__); _running = false; break; + case SESSION_LOCKED_ACTION: + vd_printf("%s: received session locked event", __func__); + _session_is_locked = true; + break; + case SESSION_UNLOCKED_ACTION: + vd_printf("%s: received session unlocked event", __func__); + _session_is_locked = false; + break; default: vd_printf("%s: action not handled (%d)", __func__, action); _running = false; @@ -1516,10 +1542,6 @@ LRESULT CALLBACK VDAgent::wnd_proc(HWND hwnd, UINT message, WPARAM wparam, LPARA case WM_WTSSESSION_CHANGE: if (wparam == WTS_SESSION_LOGON) { a->set_control_event(CONTROL_LOGON); - } else if (wparam == WTS_SESSION_LOCK) { - a->_session_is_locked = true; - } else if (wparam == WTS_SESSION_UNLOCK) { - a->_session_is_locked = false; } break; default: diff --git a/vdservice/vdservice.cpp b/vdservice/vdservice.cpp index 1892b72..9a11d87 100644 --- a/vdservice/vdservice.cpp +++ b/vdservice/vdservice.cpp @@ -91,6 +91,8 @@ private: PROCESS_INFORMATION _agent_proc_info; HANDLE _control_event; HANDLE _agent_stop_event; + HANDLE _session_locked_event; + HANDLE _session_unlocked_event; HANDLE* _events; TCHAR _agent_path[MAX_PATH]; VDControlQueue _control_queue; @@ -103,6 +105,7 @@ private: int _system_version; bool _agent_alive; bool _running; + bool _session_is_locked; VDLog* _log; unsigned _events_count; }; @@ -116,6 +119,7 @@ VDService::VDService() , _agent_restarts (0) , _agent_alive (false) , _running (false) + , _session_is_locked (false) , _log (NULL) , _events_count(0) { @@ -123,11 +127,15 @@ VDService::VDService() _system_version = supported_system_version(); _control_event = CreateEvent(NULL, FALSE, FALSE, NULL); _agent_stop_event = CreateEvent(NULL, FALSE, FALSE, VD_AGENT_STOP_EVENT); + _session_locked_event = CreateEvent(NULL, FALSE, FALSE, VD_AGENT_SESSION_LOCKED_EVENT); + _session_unlocked_event = CreateEvent(NULL, FALSE, FALSE, VD_AGENT_SESSION_UNLOCKED_EVENT); _agent_path[0] = wchar_t('\0'); } VDService::~VDService() { + CloseHandle(_session_unlocked_event); + CloseHandle(_session_locked_event); CloseHandle(_agent_stop_event); CloseHandle(_control_event); delete[] _events; @@ -290,6 +298,12 @@ DWORD WINAPI VDService::control_handler(DWORD control, DWORD event_type, LPVOID if (event_type == WTS_CONSOLE_CONNECT) { s->_session_id = session_id; s->set_control_event(VD_CONTROL_RESTART_AGENT); + } else if (event_type == WTS_SESSION_LOCK) { + s->_session_is_locked = true; + SetEvent(s->_session_locked_event); + } else if (event_type == WTS_SESSION_UNLOCK) { + s->_session_is_locked = false; + SetEvent(s->_session_unlocked_event); } break; } @@ -728,6 +742,12 @@ bool VDService::launch_agent() return false; } _agent_alive = true; + + if (!_session_is_locked) { + /* For security reasons, VDAgent always starts thinking its session is + * locked. We should notify that's not true */ + SetEvent(_session_unlocked_event); + } return true; } -- 2.9.3 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel