git.iko.soy/unity8
on xenial help sign in
Fix a possible crash in our PAM threading code. (LP: #1425362)
7b95d89Michael Terry
plugins/LightDM/liblightdm/GreeterPrivate.cpp+29−5
compare with xenial
unifiedsplit
@@ -56,7 +56,8 @@ public:
56 this, SLOT(handleMessage(pam_handle *, QString, QLightDM::Greeter::MessageType)));
56 this, SLOT(handleMessage(pam_handle *, QString, QLightDM::Greeter::MessageType)));
57 // This next connect is how we pass ResponseFutures between threads
57 // This next connect is how we pass ResponseFutures between threads
58 connect(this, SIGNAL(showPrompt(pam_handle *, QString, QLightDM::Greeter::PromptType, QLightDM::GreeterImpl::ResponseFuture)),
58 connect(this, SIGNAL(showPrompt(pam_handle *, QString, QLightDM::Greeter::PromptType, QLightDM::GreeterImpl::ResponseFuture)),
59− this, SLOT(handlePrompt(pam_handle *, QString, QLightDM::Greeter::PromptType, QLightDM::GreeterImpl::ResponseFuture)));
59+ this, SLOT(handlePrompt(pam_handle *, QString, QLightDM::Greeter::PromptType, QLightDM::GreeterImpl::ResponseFuture)),
60+ Qt::BlockingQueuedConnection);
60 }
61 }
61
62
62 ~GreeterImpl()
63 ~GreeterImpl()
@@ -68,6 +69,13 @@ public:
68 {
69 {
69 // Clear out any existing PAM interactions first
70 // Clear out any existing PAM interactions first
70 cancelPam();
71 cancelPam();
72+ if (pamHandle != nullptr) {
73+ // While we were cancelling pam above, we processed Qt events.
74+ // Which may have allowed someone to call start() on us again.
75+ // In which case, we'll bail on our current start() call.
76+ // This isn't racy because it's all in the same thread.
77+ return;
78+ }
71
79
72 AppData *appData = new AppData();
80 AppData *appData = new AppData();
73 appData->impl = this;
81 appData->impl = this;
@@ -79,7 +87,7 @@ public:
79
87
80 if (pam_start("lightdm", username.toUtf8(), &conversation, &pamHandle) == PAM_SUCCESS) {
88 if (pam_start("lightdm", username.toUtf8(), &conversation, &pamHandle) == PAM_SUCCESS) {
81 appData->handle = pamHandle;
89 appData->handle = pamHandle;
82− futureWatcher.setFuture(QtConcurrent::run(authenticateWithPam, pamHandle));
90+ futureWatcher.setFuture(QtConcurrent::mapped(QList<pam_handle*>() << pamHandle, authenticateWithPam));
83 } else {
91 } else {
84 delete appData;
92 delete appData;
85 greeterPrivate->authenticated = false;
93 greeterPrivate->authenticated = false;
@@ -88,7 +96,7 @@ public:
88 }
96 }
89 }
97 }
90
98
91− static int authenticateWithPam(pam_handle* pamHandle)
99+ static int authenticateWithPam(pam_handle* const& pamHandle)
92 {
100 {
93 int pamStatus = pam_authenticate(pamHandle, 0);
101 int pamStatus = pam_authenticate(pamHandle, 0);
94 if (pamStatus == PAM_SUCCESS) {
102 if (pamStatus == PAM_SUCCESS) {
@@ -242,11 +250,22 @@ private Q_SLOTS:
242private:
250private:
243 void cancelPam()
251 void cancelPam()
244 {
252 {
245− // Unfortunately we can't simply cancel our QFuture because QtConcurrent::run doesn't support cancel
246 if (pamHandle != nullptr) {
253 if (pamHandle != nullptr) {
254+ QFuture<int> pamFuture = futureWatcher.future();
247 pam_handle *handle = pamHandle;
255 pam_handle *handle = pamHandle;
248 pamHandle = nullptr; // to disable normal finishPam() handling
256 pamHandle = nullptr; // to disable normal finishPam() handling
257+ pamFuture.cancel();
258+
259+ // Note the empty loop, we just want to clear the futures queue.
260+ // Any further prompts from the pam thread will be immediately
261+ // responded to/dismissed in handlePrompt().
249− while (respond(QString())); // clear our local queue of QFutures
262+ while (respond(QString()));
263+
264+ // Now let signal/slot handling happen so the thread can finish
265+ while (!pamFuture.isFinished()) {
266+ QCoreApplication::processEvents();
267+ }
268+
250 pam_end(handle, PAM_CONV_ERR);
269 pam_end(handle, PAM_CONV_ERR);
251 }
270 }
252 }
271 }
@@ -266,6 +285,11 @@ GreeterPrivate::GreeterPrivate(Greeter* parent)
266{
285{
267}
286}
268
287
288+GreeterPrivate::~GreeterPrivate()
289+{
290+ delete m_impl;
291+}
292+
269void GreeterPrivate::handleAuthenticate()
293void GreeterPrivate::handleAuthenticate()
270{
294{
271 m_impl->start(authenticationUser);
295 m_impl->start(authenticationUser);
plugins/LightDM/liblightdm/GreeterPrivate.h+1−1
@@ -30,7 +30,7 @@ class GreeterPrivate
30{
30{
31public:
31public:
32 explicit GreeterPrivate(Greeter* parent=0);
32 explicit GreeterPrivate(Greeter* parent=0);
33− virtual ~GreeterPrivate() = default;
33+ virtual ~GreeterPrivate();
34
34
35 // These variables may not be used by all subclasses, that's no problem
35 // These variables may not be used by all subclasses, that's no problem
36 bool authenticated;
36 bool authenticated;
tests/plugins/LightDM/CMakeLists.txt+35−16
@@ -1,8 +1,38 @@
1+add_definitions(
2+ -DCURRENT_SOURCE_DIR="${CMAKE_CURRENT_SOURCE_DIR}"
3+ )
4+include_directories(
5+ ${CMAKE_CURRENT_BINARY_DIR}
6+ )
7+
1add_executable(GreeterDBusTestExec
8add_executable(GreeterDBusTestExec
2 dbus.cpp
9 dbus.cpp
3 ${CMAKE_SOURCE_DIR}/plugins/LightDM/Greeter.cpp
10 ${CMAKE_SOURCE_DIR}/plugins/LightDM/Greeter.cpp
4 )
11 )
5qt5_use_modules(GreeterDBusTestExec Core DBus Quick Test)
12qt5_use_modules(GreeterDBusTestExec Core DBus Quick Test)
13+target_link_libraries(GreeterDBusTestExec
14+ -L${CMAKE_BINARY_DIR}/tests/mocks/LightDM/liblightdm
15+ -llightdm-qt5-2
16+ )
17+target_include_directories(GreeterDBusTestExec PUBLIC
18+ ${CMAKE_SOURCE_DIR}/plugins/LightDM
19+ ${CMAKE_SOURCE_DIR}/tests/mocks/LightDM
20+ )
21+add_binary_qml_test(GreeterDBus "${CMAKE_BINARY_DIR}/tests/mocks/LightDM/liblightdm" MockLightDM "QML2_IMPORT_PATH=${CMAKE_BINARY_DIR}/tests/mocks")
22+
23+add_executable(GreeterPamTestExec
24+ pam.cpp
25+ ${CMAKE_SOURCE_DIR}/plugins/LightDM/liblightdm/GreeterPrivate.cpp
26+ )
27+qt5_use_modules(GreeterPamTestExec Concurrent Core Test)
28+target_link_libraries(GreeterPamTestExec
29+ integratedLightDM
30+ )
31+target_include_directories(GreeterPamTestExec PUBLIC
32+ ${CMAKE_SOURCE_DIR}/plugins/LightDM/liblightdm
33+ )
34+add_qmltest_target(testGreeterPam "${CMAKE_CURRENT_BINARY_DIR}/GreeterPamTestExec" TRUE FALSE)
35+add_dependencies(testGreeterPam GreeterPamTestExec)
6
36
7add_executable(GreeterUsersModelTestExec
37add_executable(GreeterUsersModelTestExec
8 usersmodel.cpp
38 usersmodel.cpp
@@ -10,24 +40,13 @@ add_executable(GreeterUsersModelTestExec
10 ${CMAKE_SOURCE_DIR}/plugins/Utils/unitysortfilterproxymodelqml.cpp
40 ${CMAKE_SOURCE_DIR}/plugins/Utils/unitysortfilterproxymodelqml.cpp
11 )
41 )
12qt5_use_modules(GreeterUsersModelTestExec Core Test)
42qt5_use_modules(GreeterUsersModelTestExec Core Test)
13−
43+target_link_libraries(GreeterUsersModelTestExec
14−include_directories(
44+ -L${CMAKE_BINARY_DIR}/tests/mocks/LightDM/liblightdm
15− ${CMAKE_CURRENT_BINARY_DIR}
45+ -llightdm-qt5-2
46+ )
47+target_include_directories(GreeterUsersModelTestExec PUBLIC
16 ${CMAKE_SOURCE_DIR}/plugins/LightDM
48 ${CMAKE_SOURCE_DIR}/plugins/LightDM
17 ${CMAKE_SOURCE_DIR}/plugins/Utils
49 ${CMAKE_SOURCE_DIR}/plugins/Utils
18 ${CMAKE_SOURCE_DIR}/tests/mocks/LightDM
50 ${CMAKE_SOURCE_DIR}/tests/mocks/LightDM
19 )
51 )
20−
21−target_link_libraries(GreeterDBusTestExec
22− MockLightDM
23− )
24−
25−target_link_libraries(GreeterUsersModelTestExec
26− MockLightDM
27− )
28−
29−add_definitions(-DCURRENT_SOURCE_DIR="${CMAKE_CURRENT_SOURCE_DIR}")
30−
31−add_binary_qml_test(GreeterDBus "${CMAKE_BINARY_DIR}/tests/mocks/LightDM/liblightdm" MockLightDM "QML2_IMPORT_PATH=${CMAKE_BINARY_DIR}/tests/mocks")
32−
33add_binary_qml_test(GreeterUsersModel "${CMAKE_BINARY_DIR}/tests/mocks/LightDM/liblightdm" MockLightDM "LIBLIGHTDM_MOCK_MODE=full")
52add_binary_qml_test(GreeterUsersModel "${CMAKE_BINARY_DIR}/tests/mocks/LightDM/liblightdm" MockLightDM "LIBLIGHTDM_MOCK_MODE=full")
tests/plugins/LightDM/pam.cppnew file+52
@@ -0,0 +1,52 @@
1+/*
2+ * Copyright (C) 2015 Canonical, Ltd.
3+ *
4+ * This program is free software; you can redistribute it and/or modify
5+ * it under the terms of the GNU General Public License as published by
6+ * the Free Software Foundation; version 3.
7+ *
8+ * This program is distributed in the hope that it will be useful,
9+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
10+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11+ * GNU General Public License for more details.
12+ *
13+ * You should have received a copy of the GNU General Public License
14+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
15+ */
16+
17+#include "GreeterPrivate.h"
18+
19+#include <QtTest>
20+
21+class GreeterPamTest : public QObject
22+{
23+ Q_OBJECT
24+
25+private Q_SLOTS:
26+
27+ void init()
28+ {
29+ m_greeterpriv = new QLightDM::GreeterPrivate();
30+ }
31+
32+ void cleanup()
33+ {
34+ delete m_greeterpriv;
35+ QTRY_COMPARE(QThreadPool::globalInstance()->activeThreadCount(), 0);
36+ }
37+
38+ void testRapidFireAuthentication()
39+ {
40+ m_greeterpriv->authenticationUser = qgetenv("USER");
41+ for (int i = 0; i < 100; i++) {
42+ m_greeterpriv->handleAuthenticate();
43+ }
44+ }
45+
46+private:
47+ QLightDM::GreeterPrivate *m_greeterpriv;
48+};
49+
50+QTEST_GUILESS_MAIN(GreeterPamTest)
51+
52+#include "pam.moc"