diff --git a/src/glrenderer.h b/src/glrenderer.h index 85332c0..2897c04 100644 --- a/src/glrenderer.h +++ b/src/glrenderer.h @@ -317,12 +317,15 @@ class GLRenderer : public QQuickFramebufferObject GLuint texture; uint32_t width, height; uint32_t lastSeenFrame; - size_t dataLen; // track for replacement detection + uint64_t generation; // per-image stamp from ghostty; changed gen ⇒ re-upload }; QHash m_kittyTextures; uint32_t m_kittyFrameCounter = 0; static const int MAX_KITTY_TEXTURES = 32; static const int KITTY_EVICTION_FRAMES = 120; + // Queried lazily on the render thread (first kitty upload) to clamp + // image dimensions against GL_MAX_TEXTURE_SIZE before allocation. + GLint m_maxTextureSize = 0; // Snapshot of kitty placement data (built in synchronize on GUI thread, // consumed in render on render thread — avoids ghostty_terminal_get @@ -336,6 +339,7 @@ class GLRenderer : public QQuickFramebufferObject bool imageExists = false; bool needsUpload = false; size_t dataLen = 0; + uint64_t generation = 0; GhosttyKittyImageFormat format = GHOSTTY_KITTY_IMAGE_FORMAT_RGBA; QByteArray pixelData; // deep copy, only populated when needsUpload }; diff --git a/src/glrenderer_kitty.cpp b/src/glrenderer_kitty.cpp index f3f6a22..cf7a5d7 100644 --- a/src/glrenderer_kitty.cpp +++ b/src/glrenderer_kitty.cpp @@ -127,11 +127,17 @@ void GLRenderer::Renderer::snapshotKittyGraphics(GhosttyTerminal terminal, Ghost ghostty_kitty_graphics_placement_get(iter, GHOSTTY_KITTY_GRAPHICS_PLACEMENT_DATA_Y_OFFSET, &snap.yOffset); + // Ghostty exposes a per-image generation stamp (GHOSTTY_KITTY_IMAGE_DATA_GENERATION) + // that changes whenever pixel contents change — even when dimensions, format, + // and data length are identical (e.g. an app re-uploading under the same image + // ID). Keying staleness on dataLen misses those updates and renders stale pixels. + uint64_t gen = 0; + ghostty_kitty_graphics_image_get(image, GHOSTTY_KITTY_IMAGE_DATA_GENERATION, &gen); + snap.generation = gen; + if (m_kittyTextures.contains(snap.imageId)) { - size_t checkPixelsLen = 0; - ghostty_kitty_graphics_image_get(image, GHOSTTY_KITTY_IMAGE_DATA_DATA_LEN, &checkPixelsLen); - if (checkPixelsLen != m_kittyTextures[snap.imageId].dataLen) { - // Image ID reused with different data — queue old texture for deletion + if (gen != m_kittyTextures[snap.imageId].generation) { + // Image ID reused, or contents changed — queue old texture for deletion m_kittyTexturesToDelete.append(m_kittyTextures[snap.imageId].texture); m_kittyTextures.remove(snap.imageId); snap.needsUpload = true; @@ -199,6 +205,22 @@ void GLRenderer::Renderer::drawKittyImageLayer(GhosttyKittyPlacementLayer layer, if (imgW == 0 || imgH == 0) continue; + // Clamp against GL_MAX_TEXTURE_SIZE (queried lazily on the render thread). + // Without this, a malicious/buggy VT can request enormous dimensions and + // overflow size_t (4*W*H wraps on 32-bit ARM) in the gray→RGBA conversion + // below, causing a heap over-read before glTexImage2D ever rejects it. + if (m_maxTextureSize == 0) { + glGetIntegerv(GL_MAX_TEXTURE_SIZE, &m_maxTextureSize); + if (m_maxTextureSize <= 0) + m_maxTextureSize = 2048; // sane fallback if the query fails + } + if (imgW > static_cast(m_maxTextureSize) + || imgH > static_cast(m_maxTextureSize)) { + qWarning() << "Kitty image" << snap.imageId << "dimensions" << imgW << "x" << imgH + << "exceed GL_MAX_TEXTURE_SIZE" << m_maxTextureSize << "; skipping upload"; + continue; + } + const auto &info = snap.renderInfo; uint32_t xOffset = snap.xOffset, yOffset = snap.yOffset; @@ -211,6 +233,22 @@ void GLRenderer::Renderer::drawKittyImageLayer(GhosttyKittyPlacementLayer layer, GhosttyKittyImageFormat fmt = snap.format; + // Validate the source buffer has enough bytes for the declared + // format/dimensions before any read (gray→RGBA loop or glTexImage2D). + // Guards against truncated payloads and, with the clamp above, the + // size_t overflow case on 32-bit ARM. + size_t srcBpp = 4; + if (fmt == GHOSTTY_KITTY_IMAGE_FORMAT_RGB) srcBpp = 3; + else if (fmt == GHOSTTY_KITTY_IMAGE_FORMAT_GRAY) srcBpp = 1; + else if (fmt == GHOSTTY_KITTY_IMAGE_FORMAT_GRAY_ALPHA) srcBpp = 2; + size_t requiredSrcBytes = static_cast(imgW) * static_cast(imgH) * srcBpp; + if (pixelsLen < requiredSrcBytes) { + qWarning() << "Kitty image" << snap.imageId << "buffer truncated:" << pixelsLen + << "bytes, need" << requiredSrcBytes << "for" << imgW << "x" << imgH + << "; skipping upload"; + continue; + } + GLenum glFmt = GL_RGBA; bool convertedPixels = false; if (fmt == GHOSTTY_KITTY_IMAGE_FORMAT_RGB) @@ -251,8 +289,17 @@ void GLRenderer::Renderer::drawKittyImageLayer(GhosttyKittyPlacementLayer layer, glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); + // GL_UNPACK_ALIGNMENT defaults to 4, but the GL_RGB path uploads + // tightly-packed rows (width*3 bytes). For any width not divisible by + // 4, GL would read past each row's end — over-reading the heap buffer. + // Alignment=1 is correct for all our formats here (RGBA is 4-byte + // aligned regardless). + GLint prevUnpackAlignment = 4; + glGetIntegerv(GL_UNPACK_ALIGNMENT, &prevUnpackAlignment); + glPixelStorei(GL_UNPACK_ALIGNMENT, 1); glTexImage2D(GL_TEXTURE_2D, 0, glFmt, imgW, imgH, 0, glFmt, GL_UNSIGNED_BYTE, pixels); + glPixelStorei(GL_UNPACK_ALIGNMENT, prevUnpackAlignment); glBindTexture(GL_TEXTURE_2D, 0); if (convertedPixels) free(const_cast(pixels)); @@ -262,7 +309,7 @@ void GLRenderer::Renderer::drawKittyImageLayer(GhosttyKittyPlacementLayer layer, cached.width = imgW; cached.height = imgH; cached.lastSeenFrame = m_kittyFrameCounter; - cached.dataLen = pixelsLen; + cached.generation = snap.generation; m_kittyTextures.insert(snap.imageId, cached); } else if (m_kittyTextures.contains(snap.imageId)) { m_kittyTextures[snap.imageId].lastSeenFrame = m_kittyFrameCounter; diff --git a/src/ptymanager.cpp b/src/ptymanager.cpp index c0e00ef..2a79fb5 100644 --- a/src/ptymanager.cpp +++ b/src/ptymanager.cpp @@ -434,6 +434,11 @@ void PtyManager::stop(bool synchronous) m_readerThread->deleteLater(); } else { qWarning() << "PtyReaderThread did not exit after fd close; async cleanup"; + // Detach from parent so ~PtyManager doesn't try to delete a + // still-running QThread (would abort: "QThread: Destroyed + // while thread is still running"). The finished→deleteLater + // connection below owns lifecycle once the thread exits. + m_readerThread->setParent(nullptr); connect(m_readerThread, &QThread::finished, m_readerThread, &QObject::deleteLater); } m_readerThread = nullptr;