From ef8636cbebd15c7665e839136dbbf4353f23a8f0 Mon Sep 17 00:00:00 2001 From: Geoffrey McRae Date: Mon, 24 Aug 2026 12:37:00 +1000 Subject: [PATCH] [client] lgmp: snapshot and validate frame layouts --- client/include/interface/transport.h | 95 +++++++++++++ client/renderers/OpenGL/opengl.c | 10 +- client/src/main.c | 33 ++++- client/transports/LGMP/lgmp.c | 196 +++++++++++++++++---------- 4 files changed, 253 insertions(+), 81 deletions(-) diff --git a/client/include/interface/transport.h b/client/include/interface/transport.h index 5c692327..5a1ebabf 100644 --- a/client/include/interface/transport.h +++ b/client/include/interface/transport.h @@ -21,6 +21,7 @@ #ifndef _H_LG_CLIENT_TRANSPORT_ #define _H_LG_CLIENT_TRANSPORT_ +#include #include #include #include @@ -226,6 +227,100 @@ typedef struct LG_TransportFrameFormat } LG_TransportFrameFormat; +/* + * Validate the common packed-frame layout used by renderers. The optional + * dataSize result is the validated byte capacity required for the frame data. + */ +static inline bool lgTransport_validateFrameFormat( + const LG_TransportFrameFormat * format, LG_TransportFrameFlags flags, + size_t * dataSize) +{ + if (!format) + return false; + + uint32_t storageBpp; + switch (format->type) + { + case FRAME_TYPE_BGRA: + case FRAME_TYPE_RGBA: + case FRAME_TYPE_RGBA10: + case FRAME_TYPE_BGR_32: + storageBpp = 4; + break; + + case FRAME_TYPE_RGBA16F: + storageBpp = 8; + break; + + case FRAME_TYPE_RGB_24: + storageBpp = 3; + break; + + default: + return false; + } + + switch (format->rotation) + { + case FRAME_ROT_0: + case FRAME_ROT_90: + case FRAME_ROT_180: + case FRAME_ROT_270: + break; + + default: + return false; + } + + if (!format->screenWidth || !format->screenHeight || + !format->dataWidth || !format->dataHeight || + !format->frameWidth || !format->frameHeight || + !format->stride || !format->pitch || + format->screenWidth > INT_MAX || format->screenHeight > INT_MAX || + format->dataWidth > INT_MAX || format->dataHeight > INT_MAX || + format->frameWidth > INT_MAX || format->frameHeight > INT_MAX || + format->stride > INT_MAX || format->pitch > INT_MAX) + return false; + + const uint64_t rowBytes = (uint64_t)format->stride * storageBpp; + const uint64_t size = (uint64_t)format->dataHeight * format->pitch; + if (format->dataWidth > format->stride || rowBytes != format->pitch || + size > INT_MAX || format->dataHeight > format->frameHeight || + (format->dataHeight < format->frameHeight && + !(flags & LG_TRANSPORT_FRAME_TRUNCATED))) + return false; + + if (format->type == FRAME_TYPE_BGR_32) + { + if (format->dataWidth != format->stride || + (uint64_t)format->frameWidth * 3U > format->pitch) + return false; + } + else if (format->dataWidth != format->frameWidth) + return false; + + if (dataSize) + *dataSize = (size_t)size; + return true; +} + +static inline bool lgTransport_frameLayoutMatches( + const LG_TransportFrameFormat * left, + const LG_TransportFrameFormat * right) +{ + return left && right && + left->type == right->type && + left->screenWidth == right->screenWidth && + left->screenHeight == right->screenHeight && + left->dataWidth == right->dataWidth && + left->dataHeight == right->dataHeight && + left->frameWidth == right->frameWidth && + left->frameHeight == right->frameHeight && + left->rotation == right->rotation && + left->stride == right->stride && + left->pitch == right->pitch; +} + typedef struct LG_TransportFrame { uint64_t serial; diff --git a/client/renderers/OpenGL/opengl.c b/client/renderers/OpenGL/opengl.c index 8eac8b70..395edb2c 100644 --- a/client/renderers/OpenGL/opengl.c +++ b/client/renderers/OpenGL/opengl.c @@ -927,9 +927,8 @@ static enum ConfigStatus configure(struct Inst * this) * as OpenGL supports BGR directly we need to correct dimensions in order * to make this happen. */ - this->format.dataWidth = this->format.frameWidth; - this->format.dataHeight = this->format.frameHeight; - this->format.bpp = 24; + this->format.dataWidth = this->format.frameWidth; + this->format.bpp = 24; break; default: @@ -938,7 +937,8 @@ static enum ConfigStatus configure(struct Inst * this) } // calculate the texture size in bytes - this->texSize = this->format.dataHeight * this->format.pitch; + this->texSize = + (size_t)this->format.dataHeight * (size_t)this->format.pitch; this->texPos = 0; g_gl_dynProcs.glGenBuffers(BUFFER_COUNT, this->vboID); @@ -1377,7 +1377,7 @@ static bool drawFrame(struct Inst * this, 0, 0, this->format.frameWidth , - this->format.frameHeight, + this->format.dataHeight, this->vboFormat, this->dataFormat, (void*)0 diff --git a/client/src/main.c b/client/src/main.c index 8c58ada0..64ddfb4b 100644 --- a/client/src/main.c +++ b/client/src/main.c @@ -1382,6 +1382,7 @@ int main_frameThread(void * unused) { uint64_t frameSerial = 0; uint32_t formatVersion = 0; + LG_TransportFrameFormat acceptedFormat = {}; LG_RendererFormat rendererFormat; uint64_t sourceGeneration = 0; @@ -1454,6 +1455,30 @@ int main_frameThread(void * unused) primaryWorkerFailed(); break; } + if (!lgTransport_validateFrameFormat(format, frame.flags, NULL)) + { + videoPayloadRelease(); + DEBUG_ERROR( + "Transport returned an invalid frame layout: type=%d " + "data=%ux%u frame=%ux%u stride=%u pitch=%u flags=%#x", + format->type, format->dataWidth, format->dataHeight, + format->frameWidth, format->frameHeight, + format->stride, format->pitch, frame.flags); + g_state.videoOps->frame->releaseFrame(g_state.transport.handle, &frame); + primaryWorkerFailed(); + break; + } + if (!sourceChanged && g_state.formatValid && + format->version == formatVersion && + !lgTransport_frameLayoutMatches(format, &acceptedFormat)) + { + videoPayloadRelease(); + DEBUG_ERROR( + "Transport changed the frame layout without changing its version"); + g_state.videoOps->frame->releaseFrame(g_state.transport.handle, &frame); + primaryWorkerFailed(); + break; + } const bool formatChanged = sourceChanged || !g_state.formatValid || format->version != formatVersion; bool rendererSupportsNativeHDR = false; @@ -1581,10 +1606,11 @@ int main_frameThread(void * unused) for (uint32_t i = 0; !invalidDamage && i < damageCount; ++i) { const FrameDamageRect * rect = &frame.damageRects[i]; - invalidDamage = rect->x > format->frameWidth || - rect->y > format->frameHeight || + invalidDamage = !rect->width || !rect->height || + rect->x > format->frameWidth || + rect->y > format->dataHeight || rect->width > format->frameWidth - rect->x || - rect->height > format->frameHeight - rect->y; + rect->height > format->dataHeight - rect->y; } if (invalidDamage) { @@ -1630,6 +1656,7 @@ int main_frameThread(void * unused) { g_state.formatValid = true; formatVersion = format->version; + memcpy(&acceptedFormat, format, sizeof(acceptedFormat)); #ifdef ENABLE_TESTS atomic_store_explicit(&l_testFrameType, format->type, memory_order_release); diff --git a/client/transports/LGMP/lgmp.c b/client/transports/LGMP/lgmp.c index 49fc96bc..ab41f33a 100644 --- a/client/transports/LGMP/lgmp.c +++ b/client/transports/LGMP/lgmp.c @@ -52,6 +52,7 @@ struct DMAFrameInfo { const KVMFRFrame * frame; + size_t offset; size_t dataSize; int fd; }; @@ -63,6 +64,7 @@ struct LGMPFrameLease PLGMPClientQueue * subscription; PLGMPClientQueue queue; const KVMFRFrame * frame; + KVMFRFrame snapshot; LG_TransportFrameFormat format; uint64_t handle; uint32_t generation; @@ -865,6 +867,7 @@ static void lgmp_closeDMA(struct LG_Transport * this) close(this->dma[i].fd); this->dma[i].fd = -1; this->dma[i].frame = NULL; + this->dma[i].offset = 0; this->dma[i].dataSize = 0; } } @@ -1598,7 +1601,10 @@ struct LGMPFrameMessage { PLGMPClientQueue queue; LGMPMessage message; + const KVMFRFrame * sharedFrame; const KVMFRFrame * frame; + LG_TransportFrameFormat format; + size_t dataSize; struct LGMPFrameLease * lease; bool owner; }; @@ -1649,6 +1655,7 @@ static LG_TransportStatus lgmp_doneFrameMessage( const LGMP_STATUS status = lgmpClientMessageDone(message->queue); message->queue = NULL; + message->sharedFrame = NULL; message->frame = NULL; if (status == LGMP_OK) @@ -1711,26 +1718,99 @@ static void lgmp_mergeFrameStatus(LG_TransportStatus status, *result = status; } -static bool lgmp_validateFrameMessage(struct LGMPFrameMessage * message) +static LG_TransportFrameFlags lgmp_frameFlags(const KVMFRFrame * frame) { - if (message->message.size < sizeof(KVMFRFrame)) + LG_TransportFrameFlags flags = 0; + if (frame->flags & FRAME_FLAG_BLOCK_SCREENSAVER) + flags |= LG_TRANSPORT_FRAME_BLOCK_SCREENSAVER; + if (frame->flags & FRAME_FLAG_REQUEST_ACTIVATION) + flags |= LG_TRANSPORT_FRAME_REQUEST_ACTIVATION; + if (frame->flags & FRAME_FLAG_TRUNCATED) + flags |= LG_TRANSPORT_FRAME_TRUNCATED; + return flags; +} + +static void lgmp_snapshotFrameFormat(const KVMFRFrame * frame, + LG_TransportFrameFormat * format) +{ + *format = (LG_TransportFrameFormat) + { + .version = frame->formatVer, + .type = frame->type, + .screenWidth = frame->screenWidth, + .screenHeight = frame->screenHeight, + .dataWidth = frame->dataWidth, + .dataHeight = frame->dataHeight, + .frameWidth = frame->frameWidth, + .frameHeight = frame->frameHeight, + .rotation = frame->rotation, + .stride = frame->stride, + .pitch = frame->pitch, + .hdr = frame->flags & FRAME_FLAG_HDR, + .hdrPQ = frame->flags & FRAME_FLAG_HDR_PQ, + .hdrMetadata = frame->flags & FRAME_FLAG_HDR_METADATA, + .sdrWhiteLevel = frame->sdrWhiteLevel ? frame->sdrWhiteLevel : + KVMFR_SDR_WHITE_LEVEL_DEFAULT, + }; + + if (!format->hdrMetadata) + return; + + memcpy(format->hdrDisplayPrimary, frame->hdrDisplayPrimary, + sizeof(format->hdrDisplayPrimary)); + memcpy(format->hdrWhitePoint, frame->hdrWhitePoint, + sizeof(format->hdrWhitePoint)); + format->hdrMaxDisplayLuminance = frame->hdrMaxDisplayLuminance; + format->hdrMinDisplayLuminance = frame->hdrMinDisplayLuminance; + format->hdrMaxContentLightLevel = frame->hdrMaxContentLightLevel; + format->hdrMaxFrameAverageLightLevel = + frame->hdrMaxFrameAverageLightLevel; +} + +static bool lgmp_validateFrameMessage(LG_Transport * this, + struct LGMPFrameMessage * message) +{ + if (!message->message.mem || + message->message.size < sizeof(KVMFRFrame)) { DEBUG_ERROR("LGMP frame payload is too small"); return false; } - const KVMFRFrame * frame = (const KVMFRFrame *)message->message.mem; - const size_t frameDataSize = (size_t)frame->dataHeight * frame->pitch; - if (frame->type <= FRAME_TYPE_INVALID || frame->type >= FRAME_TYPE_MAX || - frame->offset > message->message.size - sizeof(FrameBuffer) || - frameDataSize > - message->message.size - frame->offset - sizeof(FrameBuffer)) + memcpy(&message->lease->snapshot, message->message.mem, + sizeof(message->lease->snapshot)); + const KVMFRFrame * frame = &message->lease->snapshot; + lgmp_snapshotFrameFormat(frame, &message->format); + + size_t frameDataSize; + if (!lgTransport_validateFrameFormat( + &message->format, lgmp_frameFlags(frame), &frameDataSize)) + { + DEBUG_ERROR("LGMP frame payload contains an invalid frame layout"); + return false; + } + + const size_t messageSize = message->message.size; + if (frame->offset < sizeof(KVMFRFrame) || + frame->offset > messageSize - sizeof(FrameBuffer) || + frameDataSize > messageSize - frame->offset - sizeof(FrameBuffer) || + (uintptr_t)((const uint8_t *)message->message.mem + frame->offset) % + _Alignof(FrameBuffer)) { DEBUG_ERROR("LGMP frame payload contains invalid dimensions or offsets"); return false; } - message->frame = frame; + if (this->formatValid && this->format.version == frame->formatVer && + !lgTransport_frameLayoutMatches(&this->format, &message->format)) + { + DEBUG_ERROR("LGMP frame layout changed without a format version change"); + return false; + } + + message->sharedFrame = (const KVMFRFrame *)message->message.mem; + message->frame = frame; + message->dataSize = frameDataSize; return true; } @@ -1769,15 +1849,30 @@ static void lgmp_selectNewestFrameMessage( *selected = candidate; } -static int lgmp_getDMA(struct LG_Transport * this, const KVMFRFrame * frame, - size_t dataSize) +static int lgmp_getDMA(struct LG_Transport * this, + const KVMFRFrame * sharedFrame, size_t frameOffset, size_t dataSize) { + const uintptr_t base = (uintptr_t)this->shm.mem; + const uintptr_t address = (uintptr_t)sharedFrame; + if (address < base) + return -1; + + const size_t position = address - base; + if (position > this->lgmpSize || + frameOffset > this->lgmpSize - position || + sizeof(FrameBuffer) > this->lgmpSize - position - frameOffset) + return -1; + + const size_t offset = position + frameOffset + sizeof(FrameBuffer); + if (dataSize > this->lgmpSize - offset) + return -1; + struct DMAFrameInfo * dma = NULL; for (unsigned i = 0; i < LGMP_Q_FRAME_BUFFER_LEN; ++i) - if (this->dma[i].frame == frame) + if (this->dma[i].frame == sharedFrame) { dma = &this->dma[i]; - if (dma->dataSize < dataSize && dma->fd >= 0) + if ((dma->offset != offset || dma->dataSize < dataSize) && dma->fd >= 0) { close(dma->fd); dma->fd = -1; @@ -1790,7 +1885,7 @@ static int lgmp_getDMA(struct LG_Transport * this, const KVMFRFrame * frame, if (!this->dma[i].frame) { dma = &this->dma[i]; - dma->frame = frame; + dma->frame = sharedFrame; break; } @@ -1799,21 +1894,7 @@ static int lgmp_getDMA(struct LG_Transport * this, const KVMFRFrame * frame, if (dma->fd >= 0) return dma->fd; - const uintptr_t base = (uintptr_t)this->shm.mem; - const uintptr_t address = (uintptr_t)frame; - if (address < base) - return -1; - - const size_t position = address - base; - if (position > this->lgmpSize || - frame->offset > this->lgmpSize - position || - sizeof(FrameBuffer) > this->lgmpSize - position - frame->offset) - return -1; - - const size_t offset = position + frame->offset + sizeof(FrameBuffer); - if (dataSize > this->lgmpSize - offset) - return -1; - + dma->offset = offset; dma->dataSize = dataSize; dma->fd = ivshmemGetDMABuf(&this->shm, offset, dataSize); return dma->fd; @@ -1889,14 +1970,14 @@ static LG_TransportStatus lgmp_nextFrameLocked(LG_Transport * this, bool malformed = false; LG_TransportStatus releaseFailure = LG_TRANSPORT_OK; if (sharedStatus == LG_TRANSPORT_OK && - !lgmp_validateFrameMessage(&shared)) + !lgmp_validateFrameMessage(this, &shared)) { malformed = true; releaseFailure = lgmp_doneFrameMessage(&shared); } for (unsigned i = 0; i < LGMP_Q_FRAME_LEN; ++i) if (ownerStatus[i] == LG_TRANSPORT_OK && - !lgmp_validateFrameMessage(&owner[i])) + !lgmp_validateFrameMessage(this, &owner[i])) { malformed = true; const LG_TransportStatus done = @@ -1954,7 +2035,9 @@ static LG_TransportStatus lgmp_nextFrameLocked(LG_Transport * this, return done == LG_TRANSPORT_OK ? LG_TRANSPORT_TIMEOUT : done; } - const bool providerValid = lgmp_frameTimingReady(frame); + const bool providerValid = + selected->sharedFrame->frameSerial == frame->frameSerial && + lgmp_frameTimingReady(selected->sharedFrame); const uint64_t providerStart = providerValid ? nanotime() : 0; const bool fullDamage = !this->frameSerialValid || equalSerial || @@ -1971,58 +2054,25 @@ static LG_TransportStatus lgmp_nextFrameLocked(LG_Transport * this, result->scheduleDeadlineSerial = (uint32_t)scheduleToken; result->scheduleOwner = true; } - if (frame->flags & FRAME_FLAG_BLOCK_SCREENSAVER) - result->flags |= LG_TRANSPORT_FRAME_BLOCK_SCREENSAVER; - if (frame->flags & FRAME_FLAG_REQUEST_ACTIVATION) - result->flags |= LG_TRANSPORT_FRAME_REQUEST_ACTIVATION; - if (frame->flags & FRAME_FLAG_TRUNCATED) - result->flags |= LG_TRANSPORT_FRAME_TRUNCATED; + result->flags = lgmp_frameFlags(frame); LG_TransportFrameFormat * format = &this->format; if (!this->formatValid || format->version != frame->formatVer) { - memset(format, 0, sizeof(*format)); - format->version = frame->formatVer; - format->type = frame->type; - format->screenWidth = frame->screenWidth; - format->screenHeight = frame->screenHeight; - format->dataWidth = frame->dataWidth; - format->dataHeight = frame->dataHeight; - format->frameWidth = frame->frameWidth; - format->frameHeight = frame->frameHeight; - format->rotation = frame->rotation; - format->stride = frame->stride; - format->pitch = frame->pitch; - format->hdr = frame->flags & FRAME_FLAG_HDR; - format->hdrPQ = frame->flags & FRAME_FLAG_HDR_PQ; - format->hdrMetadata = frame->flags & FRAME_FLAG_HDR_METADATA; - format->sdrWhiteLevel = frame->sdrWhiteLevel ? frame->sdrWhiteLevel : - KVMFR_SDR_WHITE_LEVEL_DEFAULT; - if (format->hdrMetadata) - { - memcpy(format->hdrDisplayPrimary, frame->hdrDisplayPrimary, - sizeof(format->hdrDisplayPrimary)); - memcpy(format->hdrWhitePoint, frame->hdrWhitePoint, - sizeof(format->hdrWhitePoint)); - format->hdrMaxDisplayLuminance = frame->hdrMaxDisplayLuminance; - format->hdrMinDisplayLuminance = frame->hdrMinDisplayLuminance; - format->hdrMaxContentLightLevel = frame->hdrMaxContentLightLevel; - format->hdrMaxFrameAverageLightLevel = - frame->hdrMaxFrameAverageLightLevel; - } + memcpy(format, &selected->format, sizeof(*format)); this->formatValid = true; } struct LGMPFrameLease * lease = selected->lease; memcpy(&lease->format, format, sizeof(lease->format)); result->format = &lease->format; - result->framebuffer = (const FrameBuffer *)((const uint8_t *)frame + - frame->offset); + result->framebuffer = (const FrameBuffer *) + ((const uint8_t *)selected->sharedFrame + frame->offset); result->dmaFD = -1; if (useDMA) { - const size_t dataSize = (size_t)format->dataHeight * format->pitch; - result->dmaFD = lgmp_getDMA(this, frame, dataSize); + result->dmaFD = lgmp_getDMA(this, selected->sharedFrame, + frame->offset, selected->dataSize); if (result->dmaFD < 0) { const LG_TransportStatus done = lgmp_doneFrameMessage(selected); @@ -2034,14 +2084,14 @@ static LG_TransportStatus lgmp_nextFrameLocked(LG_Transport * this, if (!fullDamage && frame->damageRectsCount <= KVMFR_MAX_DAMAGE_RECTS) { - result->damageRects = frame->damageRects; + result->damageRects = lease->snapshot.damageRects; result->damageRectsCount = frame->damageRectsCount; } else if (!fullDamage) DEBUG_WARN("Invalid damage rectangles, forcing a full update"); lease->queue = selected->queue; - lease->frame = frame; + lease->frame = selected->sharedFrame; if (++this->frameLeaseHandle == 0) ++this->frameLeaseHandle; lease->handle = this->frameLeaseHandle;