Skip to content

Commit 55d96e4

Browse files
dkulpclaude
andcommitted
fix(fseq): never read a frame into an UncompressedFrameData whose malloc failed
UncompressedFrameData's constructor does `m_data = (uint8_t*)malloc(sz)` and does not check the result. readFrame() tests m_data for null, but none of the five getFrame() implementations that fill the buffer do: they go straight to `read(&data->m_data[sz], toRead)` or `memcpy(data->m_data, ...)`. When the allocation fails, the read path becomes fread(NULL, 1, N, fp), which faults inside glibc's own buffer copy - a memcpy crash with no FPP frame below it, which is exactly the shape that makes these reports hard to bucket. This is reachable, not theoretical. A channel range whose start lies past the end of the sequence turns m_dataBlockSize into a ~4GB request; the range guard in V1FSEQFile::prepareRead keeps master out of that particular hole, but the allocation can still fail for its own reasons on a 512MB controller, and four of the five call sites are in the V2 handlers, which compute the size their own way. Add one guard used by all five sites: log the size that could not be allocated and hand the frame back untouched. Returning the object rather than nullptr matches the existing early-exit paths in these functions, and readFrame() already reports false for a null buffer, so callers see a frame that did not load instead of a daemon that died. Verified with a standalone harness that builds FSEQFile.cpp on its own and runs it under a capped address space so malloc genuinely fails. The control fails first: the same harness against a tree without the range guard segfaults inside fread with buf=0x0, and with this change it logs the failed allocation and returns a frame that readFrame() rejects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2ed5566 commit 55d96e4

1 file changed

Lines changed: 28 additions & 0 deletions

File tree

src/fseq/FSEQFile.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,19 @@ class UncompressedFrameData : public FSEQFile::FrameData {
728728
std::vector<std::pair<uint32_t, uint32_t>> m_ranges;
729729
};
730730

731+
// A frame whose channel buffer could not be allocated is handed back untouched:
732+
// readFrame() already reports false for a null buffer, whereas the read/memcpy
733+
// paths below would write through it. malloc() failing here is not hypothetical --
734+
// a channel range that starts past the end of the sequence turns m_dataBlockSize
735+
// into a multi-gigabyte request, which no controller can satisfy.
736+
static bool frameBufferAllocated(const UncompressedFrameData* data, uint32_t frame, uint32_t sz) {
737+
if (data->m_data != nullptr) {
738+
return true;
739+
}
740+
LogErr(VB_SEQUENCE, "Failed to allocate %u bytes of channel data for frame %d\n", sz, (int)frame);
741+
return false;
742+
}
743+
731744
void V1FSEQFile::prepareRead(const std::vector<std::pair<uint32_t, uint32_t>>& ranges, uint32_t startFrame) {
732745
m_rangesToRead.clear();
733746
m_dataBlockSize = 0;
@@ -767,6 +780,9 @@ FrameData* V1FSEQFile::getFrame(uint32_t frame) {
767780
offset += m_seqChanDataOffset;
768781

769782
UncompressedFrameData* data = new UncompressedFrameData(frame, m_dataBlockSize, m_rangesToRead);
783+
if (!frameBufferAllocated(data, frame, m_dataBlockSize)) {
784+
return data;
785+
}
770786
if (seek(offset, SEEK_SET)) {
771787
LogErr(VB_SEQUENCE, "Failed to seek to proper offset for channel data for frame %d! %" PRIu64 "\n", frame, offset);
772788
return data;
@@ -885,6 +901,9 @@ class V2NoneCompressionHandler : public V2Handler {
885901
}
886902
virtual FrameData* getFrame(uint32_t frame) override {
887903
UncompressedFrameData* data = new UncompressedFrameData(frame, m_file->m_dataBlockSize, m_file->m_rangesToRead);
904+
if (!frameBufferAllocated(data, frame, m_file->m_dataBlockSize)) {
905+
return data;
906+
}
888907
uint64_t offset = m_file->getChannelCount();
889908
offset *= frame;
890909
offset += m_seqChanDataOffset;
@@ -1509,6 +1528,9 @@ class V2ZSTDCompressionHandler : public V2CompressedHandler {
15091528
}
15101529
BulkSlot* b = bulkGetBlock(block);
15111530
UncompressedFrameData* data = new UncompressedFrameData(frame, m_file->m_dataBlockSize, m_file->m_rangesToRead);
1531+
if (!frameBufferAllocated(data, frame, m_file->m_dataBlockSize)) {
1532+
return data;
1533+
}
15121534
if (b == nullptr) {
15131535
LogErr(VB_SEQUENCE, "Failed to get block %d for frame %d\n", (int)block, (int)frame);
15141536
return data;
@@ -1579,6 +1601,9 @@ class V2ZSTDCompressionHandler : public V2CompressedHandler {
15791601
m_curFrameInBlock = 0;
15801602
}
15811603
UncompressedFrameData* data = new UncompressedFrameData(frame, m_file->m_dataBlockSize, m_file->m_rangesToRead);
1604+
if (!frameBufferAllocated(data, frame, m_file->m_dataBlockSize)) {
1605+
return data;
1606+
}
15821607

15831608
uint32_t blockStart = m_file->m_frameOffsets[m_curBlock].first;
15841609
uint64_t frameEnd = frame < blockStart ? 0 : ((uint64_t)(frame - blockStart) + 1) * m_file->getChannelCount();
@@ -1807,6 +1832,9 @@ class V2ZLIBCompressionHandler : public V2CompressedHandler {
18071832
m_stream = nullptr;
18081833
}
18091834
UncompressedFrameData* data = new UncompressedFrameData(frame, m_file->m_dataBlockSize, m_file->m_rangesToRead);
1835+
if (!frameBufferAllocated(data, frame, m_file->m_dataBlockSize)) {
1836+
return data;
1837+
}
18101838

18111839
uint32_t blockStart = m_file->m_frameOffsets[m_curBlock].first;
18121840
uint64_t frameEnd = frame < blockStart ? 0 : ((uint64_t)(frame - blockStart) + 1) * m_file->getChannelCount();

0 commit comments

Comments
 (0)