[Fix] (MG_Remote): bound user-index draws by the declared span

This commit is contained in:
2026-09-16 14:08:49 -04:00
parent 348d22a4b9
commit a021e3cc5d
4 changed files with 146 additions and 0 deletions
@@ -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{};
+28
View File
@@ -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<Uint64>(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<const MGPDrawInfo*>(payload);
CheckDrawUserIndices(info, static_cast<const MGPDrawRange*>(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<const MGPDrawRange*>(tailAt(0)), span);
userIndices = &span;
}
// P5b d1: the indirect block, in the span's place. The layout already refused a
+6
View File
@@ -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
+109
View File
@@ -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<std::uint8_t> 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<Uint64>(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<std::uint8_t> 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) {