[Spice-devel] [vdagent-win v2] VDService to notify VDAgent about session status

Victor Toso victortoso at redhat.com
Tue Dec 13 16:00:17 UTC 2016


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 at redhat.com>
Reported-by: Andrei Stepanov <astepano at redhat.com>
---
 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



More information about the Spice-devel mailing list