Revision control
Copy as Markdown
From: Jonas Karlsson <jonas.karlsson@qt.io>
Subject: [PATCH] PhysX: Fix stack-buffer-overflow in BucketPruner raycasts
precomputeRayData() in GuBucketPruner.cpp read 16 bytes via an SSE load
(_mm_loadu_ps) from the address of a PxVec3, which is only 12 bytes.
Callers that pass a bare stack PxVec3 with nothing guaranteed to follow
it -- such as CharacterController's internal ground-detection raycast,
Cct::Controller::findTouchedObject() -- trigger a 4-byte out-of-bounds
read past the object. This is caught as a stack-buffer-overflow under
AddressSanitizer and is undefined behavior in any build.
Fix by building the SSE vector from the 3 known components instead of
loading a phantom 4th float.
This is the same fix as the one made for PhysX 4.1, where the code lived
in scenequery/src/SqBucketPruner.cpp, forward ported to 5.9.0.
---
src/3rdparty/PhysX/source/geomutils/src/GuBucketPruner.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/3rdparty/PhysX/source/geomutils/src/GuBucketPruner.cpp b/src/3rdparty/PhysX/source/geomutils/src/GuBucketPruner.cpp
--- a/src/3rdparty/PhysX/source/geomutils/src/GuBucketPruner.cpp
+++ b/src/3rdparty/PhysX/source/geomutils/src/GuBucketPruner.cpp
@@ -1257,8 +1257,10 @@ namespace
#ifdef USE_SIMD
const float Half = 0.5f * maxDist;
const __m128 HalfV = _mm_load1_ps(&Half);
- const __m128 DataV = _mm_mul_ps(_mm_loadu_ps(&rayDir.x), HalfV);
- const __m128 Data2V = _mm_add_ps(_mm_loadu_ps(&rayOrig.x), DataV);
+ const __m128 rayDirV = _mm_set_ps(0.0f, rayDir.z, rayDir.y, rayDir.x);
+ const __m128 rayOrigV = _mm_set_ps(0.0f, rayOrig.z, rayOrig.y, rayOrig.x);
+ const __m128 DataV = _mm_mul_ps(rayDirV, HalfV);
+ const __m128 Data2V = _mm_add_ps(rayOrigV, DataV);
const PxU32 MaskI = 0x7fffffff;
const __m128 FDirV = _mm_and_ps(_mm_load1_ps(reinterpret_cast<const float*>(&MaskI)), DataV);
_mm_store_ps(&rayParams->mData.x, DataV);