From 26b68313b90d3aa47c9f6cfcc4c86cfc11d48a28 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Thu, 15 Sep 2016 23:16:39 -0700 Subject: [PATCH 1/3] VideoCore: Fix out-of-bounds read in ShaderSetup::ProduceDebugInfo As far as I can tell, memset was replaced by a fill without correcting the parameter type, causing an out-of-bounds array read in the Vec4 constructor. --- src/video_core/shader/shader.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/video_core/shader/shader.cpp b/src/video_core/shader/shader.cpp index 272f3ffe1..3febe739c 100644 --- a/src/video_core/shader/shader.cpp +++ b/src/video_core/shader/shader.cpp @@ -146,10 +146,8 @@ DebugData ShaderSetup::ProduceDebugInfo(const InputVertex& input, int num_ state.debug.max_opdesc_id = 0; // Setup input register table + boost::fill(state.registers.input, Math::Vec4::AssignToAll(float24::Zero())); const auto& attribute_register_map = config.input_register_map; - float24 dummy_register; - boost::fill(state.registers.input, &dummy_register); - for (unsigned i = 0; i < num_attributes; i++) state.registers.input[attribute_register_map.GetRegisterForAttribute(i)] = input.attr[i]; From 6219654ded53c680cc34f17ff89abee8187111c8 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Thu, 15 Sep 2016 23:17:48 -0700 Subject: [PATCH 2/3] Common: Remove dangerous Vec[234] array constructors They're not currently used, and it's easy to accidentally pass a single pointer argument to them, causing an out-of-bounds read. --- src/common/vector_math.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/common/vector_math.h b/src/common/vector_math.h index 2d56f168c..a57d86d88 100644 --- a/src/common/vector_math.h +++ b/src/common/vector_math.h @@ -60,7 +60,6 @@ public: } Vec2() = default; - Vec2(const T a[2]) : x(a[0]), y(a[1]) {} Vec2(const T& _x, const T& _y) : x(_x), y(_y) {} template @@ -199,7 +198,6 @@ public: } Vec3() = default; - Vec3(const T a[3]) : x(a[0]), y(a[1]), z(a[2]) {} Vec3(const T& _x, const T& _y, const T& _z) : x(_x), y(_y), z(_z) {} template @@ -405,7 +403,6 @@ public: } Vec4() = default; - Vec4(const T a[4]) : x(a[0]), y(a[1]), z(a[2]), w(a[3]) {} Vec4(const T& _x, const T& _y, const T& _z, const T& _w) : x(_x), y(_y), z(_z), w(_w) {} template From d9a904f9cbce6fb37968aad0a82a944ee8b4d2e1 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Thu, 15 Sep 2016 23:18:58 -0700 Subject: [PATCH 3/3] VideoCore: Shader interpreter cleanups --- src/video_core/shader/shader_interpreter.cpp | 74 +++++++++++--------- 1 file changed, 42 insertions(+), 32 deletions(-) diff --git a/src/video_core/shader/shader_interpreter.cpp b/src/video_core/shader/shader_interpreter.cpp index 501d00b6b..6abb6761f 100644 --- a/src/video_core/shader/shader_interpreter.cpp +++ b/src/video_core/shader/shader_interpreter.cpp @@ -116,32 +116,36 @@ void RunInterpreter(const ShaderSetup& setup, UnitState& state, unsigned : state.address_registers[instr.common.address_register_index - 1]; const float24* src1_ = LookupSourceRegister(instr.common.GetSrc1(is_inverted) + - (!is_inverted * address_offset)); + (is_inverted ? 0 : address_offset)); const float24* src2_ = LookupSourceRegister(instr.common.GetSrc2(is_inverted) + - (is_inverted * address_offset)); + (is_inverted ? address_offset : 0)); const bool negate_src1 = ((bool)swizzle.negate_src1 != false); const bool negate_src2 = ((bool)swizzle.negate_src2 != false); float24 src1[4] = { - src1_[(int)swizzle.GetSelectorSrc1(0)], src1_[(int)swizzle.GetSelectorSrc1(1)], - src1_[(int)swizzle.GetSelectorSrc1(2)], src1_[(int)swizzle.GetSelectorSrc1(3)], + src1_[(int)swizzle.src1_selector_0.Value()], + src1_[(int)swizzle.src1_selector_1.Value()], + src1_[(int)swizzle.src1_selector_2.Value()], + src1_[(int)swizzle.src1_selector_3.Value()], }; if (negate_src1) { - src1[0] = src1[0] * float24::FromFloat32(-1); - src1[1] = src1[1] * float24::FromFloat32(-1); - src1[2] = src1[2] * float24::FromFloat32(-1); - src1[3] = src1[3] * float24::FromFloat32(-1); + src1[0] = -src1[0]; + src1[1] = -src1[1]; + src1[2] = -src1[2]; + src1[3] = -src1[3]; } float24 src2[4] = { - src2_[(int)swizzle.GetSelectorSrc2(0)], src2_[(int)swizzle.GetSelectorSrc2(1)], - src2_[(int)swizzle.GetSelectorSrc2(2)], src2_[(int)swizzle.GetSelectorSrc2(3)], + src2_[(int)swizzle.src2_selector_0.Value()], + src2_[(int)swizzle.src2_selector_1.Value()], + src2_[(int)swizzle.src2_selector_2.Value()], + src2_[(int)swizzle.src2_selector_3.Value()], }; if (negate_src2) { - src2[0] = src2[0] * float24::FromFloat32(-1); - src2[1] = src2[1] * float24::FromFloat32(-1); - src2[2] = src2[2] * float24::FromFloat32(-1); - src2[3] = src2[3] * float24::FromFloat32(-1); + src2[0] = -src2[0]; + src2[1] = -src2[1]; + src2[2] = -src2[2]; + src2[3] = -src2[3]; } float24* dest = @@ -451,34 +455,40 @@ void RunInterpreter(const ShaderSetup& setup, UnitState& state, unsigned const bool negate_src3 = ((bool)swizzle.negate_src3 != false); float24 src1[4] = { - src1_[(int)swizzle.GetSelectorSrc1(0)], src1_[(int)swizzle.GetSelectorSrc1(1)], - src1_[(int)swizzle.GetSelectorSrc1(2)], src1_[(int)swizzle.GetSelectorSrc1(3)], + src1_[(int)swizzle.src1_selector_0.Value()], + src1_[(int)swizzle.src1_selector_1.Value()], + src1_[(int)swizzle.src1_selector_2.Value()], + src1_[(int)swizzle.src1_selector_3.Value()], }; if (negate_src1) { - src1[0] = src1[0] * float24::FromFloat32(-1); - src1[1] = src1[1] * float24::FromFloat32(-1); - src1[2] = src1[2] * float24::FromFloat32(-1); - src1[3] = src1[3] * float24::FromFloat32(-1); + src1[0] = -src1[0]; + src1[1] = -src1[1]; + src1[2] = -src1[2]; + src1[3] = -src1[3]; } float24 src2[4] = { - src2_[(int)swizzle.GetSelectorSrc2(0)], src2_[(int)swizzle.GetSelectorSrc2(1)], - src2_[(int)swizzle.GetSelectorSrc2(2)], src2_[(int)swizzle.GetSelectorSrc2(3)], + src2_[(int)swizzle.src2_selector_0.Value()], + src2_[(int)swizzle.src2_selector_1.Value()], + src2_[(int)swizzle.src2_selector_2.Value()], + src2_[(int)swizzle.src2_selector_3.Value()], }; if (negate_src2) { - src2[0] = src2[0] * float24::FromFloat32(-1); - src2[1] = src2[1] * float24::FromFloat32(-1); - src2[2] = src2[2] * float24::FromFloat32(-1); - src2[3] = src2[3] * float24::FromFloat32(-1); + src2[0] = -src2[0]; + src2[1] = -src2[1]; + src2[2] = -src2[2]; + src2[3] = -src2[3]; } float24 src3[4] = { - src3_[(int)swizzle.GetSelectorSrc3(0)], src3_[(int)swizzle.GetSelectorSrc3(1)], - src3_[(int)swizzle.GetSelectorSrc3(2)], src3_[(int)swizzle.GetSelectorSrc3(3)], + src3_[(int)swizzle.src3_selector_0.Value()], + src3_[(int)swizzle.src3_selector_1.Value()], + src3_[(int)swizzle.src3_selector_2.Value()], + src3_[(int)swizzle.src3_selector_3.Value()], }; if (negate_src3) { - src3[0] = src3[0] * float24::FromFloat32(-1); - src3[1] = src3[1] * float24::FromFloat32(-1); - src3[2] = src3[2] * float24::FromFloat32(-1); - src3[3] = src3[3] * float24::FromFloat32(-1); + src3[0] = -src3[0]; + src3[1] = -src3[1]; + src3[2] = -src3[2]; + src3[3] = -src3[3]; } float24* dest =