From 3d3a346eeff5dd575cd1838588da318456d7a52b Mon Sep 17 00:00:00 2001 From: Hongli Lai Date: Fri, 1 Nov 2024 17:25:13 +0100 Subject: [PATCH 1/8] Fix use-after-free and undefined behavior in Session destruction code - Use of `this` after calling the destructor is undefined behavior even though the underlying memory is still allocated. We fix this with a copy of the pointer. - Use of `context` field after calling the destructor is wrong. --- src/agent/Core/ApplicationPool/Session.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/agent/Core/ApplicationPool/Session.h b/src/agent/Core/ApplicationPool/Session.h index 98f79f30c2..2aa9cda604 100644 --- a/src/agent/Core/ApplicationPool/Session.h +++ b/src/agent/Core/ApplicationPool/Session.h @@ -109,9 +109,14 @@ class Session: public AbstractSession { } void destroySelf() const { + Context *context = this->context; + Session *storagePointer = const_cast(this); this->~Session(); + LockGuard l(context->memoryManagementSyncher); - context->sessionObjectPool.free(const_cast(this)); + // Use `storagePointer` because using `this` after calling the destructor + // is undefined behavior. + context->sessionObjectPool.free(storagePointer); } public: From 681391f9cf8afdb5b9f158a2f8e109e3fdc9f138 Mon Sep 17 00:00:00 2001 From: Hongli Lai Date: Fri, 1 Nov 2024 17:26:06 +0100 Subject: [PATCH 2/8] Run Process destructor outside of the pool memory management mutex lock --- src/agent/Core/ApplicationPool/Process.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/agent/Core/ApplicationPool/Process.h b/src/agent/Core/ApplicationPool/Process.h index 933b2cef35..c716b13f41 100644 --- a/src/agent/Core/ApplicationPool/Process.h +++ b/src/agent/Core/ApplicationPool/Process.h @@ -337,8 +337,13 @@ class Process { void destroySelf() const { Context *context = getContext(); + Process *storagePointer = const_cast(this); + this->~Process(); + LockGuard l(context->memoryManagementSyncher); - context->processObjectPool.destroy(const_cast(this)); + // Use `storagePointer` because using `this` after calling the destructor + // is undefined behavior. + context->processObjectPool.free(storagePointer); } From b2b195404dbeaa62a00726dd3e782be439beba2f Mon Sep 17 00:00:00 2001 From: Hongli Lai Date: Sun, 27 Oct 2024 15:01:37 +0100 Subject: [PATCH 3/8] Tests: fix detecting Python on systems with only "python3" command --- test/cxx/Core/ApplicationPool/PoolTest.cpp | 3 ++ test/cxx/TestSupport.cpp | 36 +++++++++++++++++++++- test/cxx/TestSupport.h | 24 +++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/test/cxx/Core/ApplicationPool/PoolTest.cpp b/test/cxx/Core/ApplicationPool/PoolTest.cpp index 313b89ecb5..fe8ba18608 100644 --- a/test/cxx/Core/ApplicationPool/PoolTest.cpp +++ b/test/cxx/Core/ApplicationPool/PoolTest.cpp @@ -23,6 +23,7 @@ namespace tut { Context context; PoolPtr pool; Pool::DebugSupportPtr debug; + string pythonCommand; Ticket ticket; GetCallback callback; SessionPtr currentSession; @@ -47,6 +48,7 @@ namespace tut { context.finalize(); pool = boost::make_shared(&context); pool->initialize(); + pythonCommand = findPythonCommand(); callback.func = _callback; callback.userData = this; @@ -113,6 +115,7 @@ namespace tut { options.spawnMethod = "dummy"; options.appRoot = "stub/rack"; options.appType = "ruby"; + options.python = pythonCommand.c_str(); options.appStartCommand = "ruby start.rb"; options.startupFile = "start.rb"; options.loadShellEnvvars = false; diff --git a/test/cxx/TestSupport.cpp b/test/cxx/TestSupport.cpp index 5955f17ea5..b624a32714 100644 --- a/test/cxx/TestSupport.cpp +++ b/test/cxx/TestSupport.cpp @@ -7,9 +7,11 @@ #include #include #include -#include +#include +#include #include #include +#include #include #include @@ -36,6 +38,38 @@ createInstanceDir(InstanceDirectoryPtr &instanceDir) { instanceDir = boost::make_shared(options); } +string +findCommandInPath(const string &basename) { + const char *path = getenv("PATH"); + if (path == nullptr) { + return string(); + } + + vector directories; + split(path, ':', directories); + + for (const string &dir: directories) { + string fullPath = dir + "/" + basename; + if (fileExists(fullPath)) { + return fullPath; + } + } + + return string(); +} + +string +findPythonCommand() { + string python = findCommandInPath("python"); + if (python.empty()) { + python = findCommandInPath("python3"); + } + if (python.empty()) { + python = findCommandInPath("python2"); + } + return python; +} + void writeUntilFull(int fd) { int flags, ret; diff --git a/test/cxx/TestSupport.h b/test/cxx/TestSupport.h index 872f2127d3..46267e4f3a 100644 --- a/test/cxx/TestSupport.h +++ b/test/cxx/TestSupport.h @@ -106,6 +106,30 @@ extern Json::Value testConfig; */ void createInstanceDir(InstanceDirectoryPtr &instanceDir); +/** + * Find the given command in PATH. Returns the empty string if not found. + * + * @throws FileSystemException + * @throws TimeRetrievalException + * @throws boost::thread_interrupted + */ +string findCommandInPath(const string &basename); + +/** + * Find an appropriate Python command for this system, regardless of whether it's Python 2 or 3. + * Returns something like "/usr/bin/python3" if possible, but on older + * systems without Python 3 it could return something like "/usr/bin/python". + * + * This function primarily exists because newer systems don't have a "python" command anymore, only "python3". + * + * Returns the empty string if not found. + * + * @throws FileSystemException + * @throws TimeRetrievalException + * @throws boost::thread_interrupted + */ +string findPythonCommand(); + /** * Writes zeroes into the given file descriptor its buffer is full (i.e. * the next write will block). From db96a1dce18cb175545d2bb6bf6330c548fe3c7c Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 11 Nov 2024 12:13:20 -0700 Subject: [PATCH 4/8] stop testing python2 and node 16 on EL8 --- packaging/rpm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/rpm b/packaging/rpm index f4477869a7..0d4a28980d 160000 --- a/packaging/rpm +++ b/packaging/rpm @@ -1 +1 @@ -Subproject commit f4477869a7e2c2df9059faeb4eedf33cd3866025 +Subproject commit 0d4a28980db26fec2d41fbb2e0f7a1dbc9840858 From 3c57fc1deecfb74e8fde5ea011eba044ee9ecd7f Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 11 Nov 2024 13:03:48 -0700 Subject: [PATCH 5/8] mark passenger repo as safe in rpm tests --- packaging/rpm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/rpm b/packaging/rpm index 0d4a28980d..e60aeb3e87 160000 --- a/packaging/rpm +++ b/packaging/rpm @@ -1 +1 @@ -Subproject commit 0d4a28980db26fec2d41fbb2e0f7a1dbc9840858 +Subproject commit e60aeb3e872a7383f7f386ef748aa51cfbf1b461 From 6188031708947d377890da027a3923da171ce766 Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 11 Nov 2024 13:21:12 -0700 Subject: [PATCH 6/8] mark all repos as safe in rpm tests --- packaging/rpm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/rpm b/packaging/rpm index e60aeb3e87..6bcc4a6af0 160000 --- a/packaging/rpm +++ b/packaging/rpm @@ -1 +1 @@ -Subproject commit e60aeb3e872a7383f7f386ef748aa51cfbf1b461 +Subproject commit 6bcc4a6af08a1b7287e3102469f234766181230c From 322970219688e34a2d1fea128c0e5e077f773899 Mon Sep 17 00:00:00 2001 From: Hongli Lai Date: Wed, 13 Nov 2024 16:55:15 +0100 Subject: [PATCH 7/8] Don't double trigger Github workflows upon pushing to pull request branch --- .github/workflows/binaries.yml | 1 - .github/workflows/debian.yml | 1 - .github/workflows/main.yml | 1 - .github/workflows/rpm.yml | 1 - 4 files changed, 4 deletions(-) diff --git a/.github/workflows/binaries.yml b/.github/workflows/binaries.yml index e2f427c066..48101887b4 100644 --- a/.github/workflows/binaries.yml +++ b/.github/workflows/binaries.yml @@ -6,7 +6,6 @@ env: on: push: {} - pull_request: {} jobs: build_linux: diff --git a/.github/workflows/debian.yml b/.github/workflows/debian.yml index f9e63a5875..9371c88878 100644 --- a/.github/workflows/debian.yml +++ b/.github/workflows/debian.yml @@ -6,7 +6,6 @@ env: on: push: {} - pull_request: {} jobs: define-matrix: diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index deb7f588e2..5934671d05 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -9,7 +9,6 @@ env: on: push: {} - pull_request: {} jobs: cxx-linux: diff --git a/.github/workflows/rpm.yml b/.github/workflows/rpm.yml index 53d854f21b..5191a3f4b3 100644 --- a/.github/workflows/rpm.yml +++ b/.github/workflows/rpm.yml @@ -6,7 +6,6 @@ env: on: push: {} - pull_request: {} jobs: define-matrix: From 181f0cf91b9e606a3bc78cdd6af5530c2673abc2 Mon Sep 17 00:00:00 2001 From: Hongli Lai Date: Wed, 13 Nov 2024 16:58:01 +0100 Subject: [PATCH 8/8] Fix uploading "Passenger generic binaries CI" Linux artifacts [ci skip] --- .github/workflows/binaries.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/binaries.yml b/.github/workflows/binaries.yml index 48101887b4..43a738fc98 100644 --- a/.github/workflows/binaries.yml +++ b/.github/workflows/binaries.yml @@ -54,7 +54,7 @@ jobs: - uses: actions/upload-artifact@v4 with: name: binaries-linux-${{ matrix.arch }} - path: packages/binaries/linux/output/**/* + path: packaging/binaries/linux/output/**/* - name: Update cache run: ./dev/ci/update-cache-az-blob-storage