diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index bcbd99ba..dcfc3783 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -408,6 +408,9 @@ namespace MobileGL::MG_Remote::Server { const MG_Pipe::MGPDrawRange* ranges, const MG_Pipe::MGHostSpan* userIndices, const MG_Pipe::MGPDrawIndirect* indirect) { + if (userIndices != nullptr) { + Wire::CheckDrawUserIndices(info, ranges, *userIndices); + } // The witness first, before the backend is consulted, so a unit process with no // backend object still sees the wire's fields (PipeApplier.h LastDraw). m_lastDraw = LastDrawRecord{}; diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 95e629a5..e44c2a36 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -558,6 +558,26 @@ namespace MobileGL::MG_Remote::Wire { } } + void CheckDrawUserIndices(const MGPDrawInfo& info, const MGPDrawRange* ranges, + const MGHostSpan& span) { + if ((info.Flags & kDrawHasUserIndices) == 0 || (info.Flags & kDrawIsIndirect) != 0 || + info.NumDraws != 1 || ranges == nullptr) { + WireProtocolFatal("DrawVbo.userIndices.shape", + "a user-index span requires exactly one direct indexed range"); + } + if (info.IndexSize != 1 && info.IndexSize != 2 && info.IndexSize != 4) { + WireProtocolFatalAt("DrawVbo.userIndices.IndexSize", info.IndexSize, 4); + } + // Client arrays are staged from their first index; Start only addresses an EBO. + if (ranges[0].Start != 0) { + WireProtocolFatalAt("DrawVbo.userIndices.Start", ranges[0].Start, 0); + } + const Uint64 required = static_cast(ranges[0].Count) * info.IndexSize; + if (required > span.Size) { + WireProtocolFatalAt("DrawVbo.userIndices.extent", required, span.Size); + } + } + // --------------------------------------------------------------------------------- // The record layout: the tail arithmetic both sides run // --------------------------------------------------------------------------------- @@ -912,6 +932,13 @@ namespace MobileGL::MG_Remote::Wire { } } + if (op == MGPWireOp::DrawVbo && layout.SecondTailIsHostSpans) { + MGHostSpan span{}; + std::memcpy(&span, tails[1].Bytes, sizeof(span)); + const auto& info = *static_cast(payload); + CheckDrawUserIndices(info, static_cast(tails[0].Bytes), span); + } + const Uint64 total = layout.TotalBytes; if (total > m_cmd->MaxRecordBytes()) { // R-10: P5 does no chunking and must instead PROVE it never needs any. This is @@ -1964,6 +1991,7 @@ namespace MobileGL::MG_Remote::Wire { // left SEG_STAGE reached the sink and that promise was false. P5b's d1 is what // arms this path (client-side index arrays staged whole, CONTRACT-P5B.md d1). CheckHostSpanIsHonest(span, *m_segments); + CheckDrawUserIndices(info, reinterpret_cast(tailAt(0)), span); userIndices = &span; } // P5b d1: the indirect block, in the span's place. The layout already refused a diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.h b/MobileGL/MG_Remote/Wire/PipeWireCodec.h index 5690b59c..00137595 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.h +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.h @@ -148,6 +148,12 @@ namespace MobileGL::MG_Remote::Wire { // when nobody will be reading this file. void CheckHostSpanIsHonest(const MG_Pipe::MGHostSpan& span, const SegmentTable& segments); + // A legal segment run must also contain every index the draw will consume. Shared by + // the encoder, decoder and sink so a direct sink call cannot bypass the extent gate. + void CheckDrawUserIndices(const MG_Pipe::MGPDrawInfo& info, + const MG_Pipe::MGPDrawRange* ranges, + const MG_Pipe::MGHostSpan& span); + // ---- one record's shape, computed ONCE and read by both sides ---------------------- // // THE TAIL CROSS-CHECK LIVES HERE AND NOWHERE ELSE (BRIEF ยง5 w1, contract table 1 group diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 07f135a4..389d5d2c 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -1004,6 +1004,34 @@ TEST_F(PipeWireCodecTest, DrawVboConditionalSpanTailIsEightAlignedAndSurvives) { EXPECT_FALSE(wire.Sink().SawIndirect); } +TEST_F(PipeWireCodecTest, UserIndexSpanExactlyAtSegmentEndReachesTheSink) { + Wire2 wire; + MGPDrawInfo info{}; + info.Mode = GL_POINTS; + info.IndexSize = 2; + info.Flags = kDrawHasUserIndices; + info.InstanceCount = 1; + info.NumDraws = 1; + MGPDrawRange range{0, 1, 0}; + MGHostSpan span{}; + span.Seg = kSegStage; + span.Offset = Wire2::kStageBytes - 2; + span.Size = 2; + const WireTail tails[] = {{&range, sizeof(range)}, {&span, sizeof(span)}}; + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::DrawVbo, &info, sizeof(info), tails, 2), + kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + ASSERT_TRUE(applied); + ASSERT_EQ(wire.Sink().DrawRanges.size(), 1u); + EXPECT_EQ(wire.Sink().DrawRanges[0].Count, 1u); + EXPECT_EQ(wire.Sink().LastSpan.Offset, span.Offset); + EXPECT_EQ(wire.Sink().LastSpan.Size, 2u); + Server::ServerVerbSink sink; + EXPECT_FALSE(sink.OnDrawVbo(info, &range, &span, nullptr)); // no backend, valid witness + EXPECT_EQ(sink.DrawRecords(), 1u); +} + // ===================================================================================== // P5b (MG_Remote/CONTRACT-P5B.md): one round trip per row the four migration packages consume // ===================================================================================== @@ -2431,6 +2459,87 @@ TEST_F(PipeWireCodecTest, AHostSpanRunPastItsSegmentIsFatalOnDrawVbo) { << r.Log; } +// The span itself is in bounds, but the driver's index count would leave it. Exercise +// the real encoder, forged decoder input, and the sink independently; none may rely on the +// preceding layer for the count/extent check. +TEST_F(PipeWireCodecTest, UserIndexSpanRejectsAnExtentBeyondItsLastTwoBytes) { + for (int side = 0; side != 3; ++side) { + SCOPED_TRACE(side); + const ChildResult r = RunInChild([side] { + Wire2 wire; + MGPDrawInfo info{}; + info.Mode = GL_TRIANGLES; + info.IndexSize = 2; + info.Flags = kDrawHasUserIndices; + info.NumDraws = 1; + info.InstanceCount = 1; + MGPDrawRange range{0, 3, 0}; + MGHostSpan span{}; + span.Seg = kSegStage; + span.Offset = Wire2::kStageBytes - 2; + span.Size = 2; + if (side == 0) { + const WireTail tails[] = {{&range, sizeof(range)}, {&span, sizeof(span)}}; + wire.Encoder().EncodeRecord(MGPWireOp::DrawVbo, &info, sizeof(info), tails, 2); + } else if (side == 1) { + std::vector tail(16 + sizeof(span), 0); + std::memcpy(tail.data(), &range, sizeof(range)); + std::memcpy(tail.data() + 16, &span, sizeof(span)); + ForgeAndDecode(wire, MGPWireOp::DrawVbo, &info, sizeof(info), tail.data(), tail.size()); + } else { + Server::ServerVerbSink sink; + sink.OnDrawVbo(info, &range, &span, nullptr); + } + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("DrawVbo.userIndices.extent"), std::string::npos) << r.Log; + } +} + +TEST_F(PipeWireCodecTest, UserIndexSpanRequiresOneIndexedRangeAndNonWrappingExtent) { + for (int side = 0; side != 2; ++side) { + for (int malformed = 0; malformed != 6; ++malformed) { + SCOPED_TRACE(side); + SCOPED_TRACE(malformed); + const ChildResult r = RunInChild([side, malformed] { + Wire2 wire; + MGPDrawInfo info{}; + info.Mode = GL_TRIANGLES; + info.IndexSize = 2; + info.Flags = kDrawHasUserIndices; + info.NumDraws = 1; + MGPDrawRange ranges[2] = {{0, 1, 0}, {0, 1, 0}}; + MGHostSpan span{}; + span.Seg = kSegStage; + span.Size = 2; + switch (malformed) { + case 0: info.IndexSize = 0; break; // arrays cannot consume user indices + case 1: info.IndexSize = 3; break; + case 2: info.NumDraws = 0; break; + case 3: info.NumDraws = 2; break; // no flattening contract for multi-draw yet + case 4: ranges[0].Start = 1; break; + case 5: info.IndexSize = 4; ranges[0].Count = 0x80000001u; span.Size = 4; break; + } + if (side == 0) { + const WireTail tails[] = { + {ranges, static_cast(info.NumDraws) * sizeof(MGPDrawRange)}, + {&span, sizeof(span)}}; + wire.Encoder().EncodeRecord(MGPWireOp::DrawVbo, &info, sizeof(info), tails, 2); + } else { + const auto bytes = info.NumDraws * sizeof(MGPDrawRange); + const auto spanAt = (bytes + 7) & ~SizeT{7}; + std::vector tail(spanAt + sizeof(span), 0); + std::memcpy(tail.data(), ranges, bytes); + std::memcpy(tail.data() + spanAt, &span, sizeof(span)); + ForgeAndDecode(wire, MGPWireOp::DrawVbo, &info, sizeof(info), tail.data(), tail.size()); + } + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("DrawVbo.userIndices."), std::string::npos) << r.Log; + } + } +} + // ---- M5 / m8 / q2 ------------------------------------------------------------------------ TEST_F(PipeWireCodecTest, ADrawThatSetsBothTheSpanAndTheIndirectFlagIsFatal) {