Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

reduce size limit for scanline files; prevent large chunkoffset allocations #824

Merged
merged 6 commits into from
Aug 28, 2020
Merged

reduce size limit for scanline files; prevent large chunkoffset allocations #824

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dfa52f4
reduce size limit for scanline files; protect against large chunkoffs…
peterhillman Aug 21, 2020
c72485f
bugfix for memory limit changes
peterhillman Aug 21, 2020
997d42c
Merge branch 'master' into memorylimit
peterhillman Aug 21, 2020
81a54c3
rearrange chunkoffset test to protect bytesperline table too
peterhillman Aug 23, 2020
43bcd29
Merge branch 'memorylimit' of https://github.com/peterhillman/openexr…
peterhillman Aug 23, 2020
03363f8
remove extraneous function declaration; tidy comments
peterhillman Aug 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions OpenEXR/IlmImf/ImfCompressor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,37 @@ newCompressor (Compression c, size_t maxScanLineSize, const Header &hdr)
}


// for a given compression type, return the number of scanlines
// compressed into a single chunk
// TODO add to API and move to ImfCompressor.cpp
int
numLinesInBuffer(Compression comp)
{
switch(comp)
{
case NO_COMPRESSION :
case RLE_COMPRESSION:
case ZIPS_COMPRESSION:
return 1;
case ZIP_COMPRESSION:
return 16;
case PIZ_COMPRESSION:
return 32;
case PXR24_COMPRESSION:
return 16;
case B44_COMPRESSION:
case B44A_COMPRESSION:
case DWAA_COMPRESSION:
return 32;
case DWAB_COMPRESSION:
return 256;

default:
throw IEX_NAMESPACE::ArgExc ("Unknown compression type");
}
}


Compressor *
newTileCompressor (Compression c,
size_t tileLineSize,
Expand Down
8 changes: 8 additions & 0 deletions OpenEXR/IlmImf/ImfCompressor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,14 @@ Compressor * newTileCompressor (Compression c,
const Header &hdr);


//-----------------------------------------------------------------
// Return the maximum number of scanlines in each chunk
// of a scanline image using the given compression scheme
//-----------------------------------------------------------------

IMF_EXPORT
int numLinesInBuffer(Compression comp);

OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_EXIT

#endif
33 changes: 1 addition & 32 deletions OpenEXR/IlmImf/ImfMisc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1848,38 +1848,7 @@ usesLongNames (const Header &header)
return false;
}

namespace
{
// for a given compression type, return the number of scanlines
// compressed into a single chunk
// TODO add to API and move to ImfCompressor.cpp
int
numLinesInBuffer(Compression comp)
{
switch(comp)
{
case NO_COMPRESSION :
case RLE_COMPRESSION:
case ZIPS_COMPRESSION:
return 1;
case ZIP_COMPRESSION:
return 16;
case PIZ_COMPRESSION:
return 32;
case PXR24_COMPRESSION:
return 16;
case B44_COMPRESSION:
case B44A_COMPRESSION:
case DWAA_COMPRESSION:
return 32;
case DWAB_COMPRESSION:
return 256;

default:
throw IEX_NAMESPACE::ArgExc ("Unknown compression type");
}
}
}


int
getScanlineChunkOffsetTableSize(const Header& header)
Expand Down
7 changes: 7 additions & 0 deletions OpenEXR/IlmImf/ImfMisc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,13 @@ bool usesLongNames (const Header &header);
IMF_EXPORT
int getChunkOffsetTableSize(const Header& header,bool deprecated_attribute=false);


//
// return the number of scanlines in each chunk of a scanlineimage for the given scheme
//
IMF_EXPORT int
numLinesInBuffer(Compression comp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is declared in ImfCompressor.h, does it need to be declared here, too?


OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_EXIT


Expand Down
22 changes: 21 additions & 1 deletion OpenEXR/IlmImf/ImfMultiPartInputFile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,9 @@ MultiPartInputFile::Data::getPart(int partNumber)
return parts[partNumber];
}


namespace{
static const int gLargeChunkTableSize = 1024*1024;
}

void
MultiPartInputFile::Data::readChunkOffsetTables(bool reconstructChunkOffsetTable)
Expand All @@ -751,8 +753,26 @@ MultiPartInputFile::Data::readChunkOffsetTables(bool reconstructChunkOffsetTable
for (size_t i = 0; i < parts.size(); i++)
{
int chunkOffsetTableSize = getChunkOffsetTableSize(parts[i]->header);

//
// avoid allocating excessive memory.
// If the chunktablesize claims to be large,
// check the file is big enough to contain the file before allocating memory
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean "file is big enough to contain the file"? And is the trick here that the read with throw an exception if the size is off? If so, it would be good to state that's the expectation.

Same comment in ImfScanLineInputFile below, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catches: Dumb typing errors. It should make more sense now

//
if(chunkOffsetTableSize > gLargeChunkTableSize)
{
Int64 pos = is->tellg();
is->seekg(pos + (chunkOffsetTableSize-1)*sizeof(Int64));
Int64 temp;
OPENEXR_IMF_INTERNAL_NAMESPACE::Xdr::read <OPENEXR_IMF_INTERNAL_NAMESPACE::StreamIO> (*is, temp);
is->seekg(pos);

}

parts[i]->chunkOffsets.resize(chunkOffsetTableSize);



for (int j = 0; j < chunkOffsetTableSize; j++)
OPENEXR_IMF_INTERNAL_NAMESPACE::Xdr::read <OPENEXR_IMF_INTERNAL_NAMESPACE::StreamIO> (*is, parts[i]->chunkOffsets[j]);

Expand Down
42 changes: 33 additions & 9 deletions OpenEXR/IlmImf/ImfScanLineInputFile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,7 @@ newLineBufferTask (TaskGroup *group,
}



static const int gLargeChunkTableSize = 1024*1024;

} // namespace

Expand All @@ -1118,25 +1118,51 @@ void ScanLineInputFile::initialize(const Header& header)
_data->minY = dataWindow.min.y;
_data->maxY = dataWindow.max.y;

Compression comp = _data->header.compression();

_data->linesInBuffer =
numLinesInBuffer (comp);

int lineOffsetSize = (dataWindow.max.y - dataWindow.min.y +
_data->linesInBuffer) / _data->linesInBuffer;

//
// avoid allocating excessive memory due to large lineOffsets table size.
// If the chunktablesize claims to be large,
// check the file is big enough to contain the file before allocating memory
// in the bytesPerLineTable and the lineOffsets table
//
if (lineOffsetSize > gLargeChunkTableSize)
{
Int64 pos = _streamData->is->tellg();
_streamData->is->seekg(pos + (lineOffsetSize-1)*sizeof(Int64));
Int64 temp;
OPENEXR_IMF_INTERNAL_NAMESPACE::Xdr::read <OPENEXR_IMF_INTERNAL_NAMESPACE::StreamIO> (*_streamData->is, temp);
_streamData->is->seekg(pos);

}


size_t maxBytesPerLine = bytesPerLineTable (_data->header,
_data->bytesPerLine);
if(maxBytesPerLine > INT_MAX)

if (maxBytesPerLine*numLinesInBuffer(comp) > INT_MAX)
{
throw IEX_NAMESPACE::InputExc("maximum bytes per scanline exceeds maximum permissible size");
}


//
// allocate compressor objects
//
for (size_t i = 0; i < _data->lineBuffers.size(); i++)
{
_data->lineBuffers[i] = new LineBuffer (newCompressor
(_data->header.compression(),
_data->lineBuffers[i] = new LineBuffer (newCompressor(comp,
maxBytesPerLine,
_data->header));
}

_data->linesInBuffer =
numLinesInBuffer (_data->lineBuffers[0]->compressor);


_data->lineBufferSize = maxBytesPerLine * _data->linesInBuffer;

Expand All @@ -1153,8 +1179,6 @@ void ScanLineInputFile::initialize(const Header& header)
_data->linesInBuffer,
_data->offsetInLineBuffer);

int lineOffsetSize = (dataWindow.max.y - dataWindow.min.y +
_data->linesInBuffer) / _data->linesInBuffer;

_data->lineOffsets.resize (lineOffsetSize);
}
Expand Down