include/tools/zcodec.hxx | 3 -- tools/source/zcodec/zcodec.cxx | 45 ++++++++++++++++++----------------------- 2 files changed, 21 insertions(+), 27 deletions(-)
New commits: commit 8f97326bdd3f42fc82aa5e1989fd03b0af1daf64 Author: Stephan Bergmann <sberg...@redhat.com> Date: Wed May 21 18:22:27 2014 +0200 So ZCodec::ReadAsynchron was wrong in using a persistent mpIStm after all The fun thing is that with the (only) call-site to ReadAsynchron in PNGReaderImpl::ImplReadIDAT (vcl/source/gdi/pngread.cxx) passing in rIStm references to stack-allocated SvMemoryStream instances, mpIStm could point to an old, destroyed instance from a previous call, but which would have been located at exactly the same stack address as the currently passed in rIStm, so the wrong mpIStm->Read call would effectively behaved exactly the same as a correct rIStm.Read call. This went unnoticed "since the beginning" until AddressSanitizer's UseAfterReturn check came along... Change-Id: I7c75ed2d36a4c24c111d88eff647816bd2c5dbca diff --git a/include/tools/zcodec.hxx b/include/tools/zcodec.hxx index c7424ab..63a5ec2 100644 --- a/include/tools/zcodec.hxx +++ b/include/tools/zcodec.hxx @@ -39,7 +39,6 @@ class TOOLS_DLLPUBLIC ZCodec State meState; bool mbStatus; bool mbFinish; - SvStream* mpIStm; sal_uInt8* mpInBuf; sal_uIntPtr mnInBufSize; sal_uIntPtr mnInToRead; diff --git a/tools/source/zcodec/zcodec.cxx b/tools/source/zcodec/zcodec.cxx index 47c97dc..2e9ad1f 100644 --- a/tools/source/zcodec/zcodec.cxx +++ b/tools/source/zcodec/zcodec.cxx @@ -41,7 +41,6 @@ ZCodec::ZCodec( sal_uIntPtr nInBufSize, sal_uIntPtr nOutBufSize ) : meState(STATE_INIT) , mbStatus(false) , mbFinish(false) - , mpIStm(NULL) , mpInBuf(NULL) , mnInBufSize(nInBufSize) , mnInToRead(0) @@ -66,7 +65,7 @@ void ZCodec::BeginCompression( int nCompressLevel, bool updateCrc, bool gzLib ) assert(meState == STATE_INIT); mbStatus = true; mbFinish = false; - mpIStm = mpOStm = NULL; + mpOStm = NULL; mnInToRead = 0xffffffff; mpInBuf = mpOutBuf = NULL; PZSTREAM->total_out = PZSTREAM->total_in = 0; @@ -249,7 +248,6 @@ long ZCodec::ReadAsynchron( SvStream& rIStm, sal_uInt8* pData, sal_uIntPtr nSize if (meState == STATE_INIT) { InitDecompress(rIStm); - mpIStm = &rIStm; } PZSTREAM->avail_out = nSize; PZSTREAM->next_out = pData; @@ -267,7 +265,7 @@ long ZCodec::ReadAsynchron( SvStream& rIStm, sal_uInt8* pData, sal_uIntPtr nSize break; } - PZSTREAM->avail_in = mpIStm->Read ( + PZSTREAM->avail_in = rIStm.Read ( PZSTREAM->next_in = mpInBuf, nInToRead); mnInToRead -= nInToRead; commit c5a603ce2469ef6a23023ff276ccd24ca316f6c2 Author: Stephan Bergmann <sberg...@redhat.com> Date: Wed May 21 18:07:14 2014 +0200 ZCodec::mpIStm is apparently(?) effectively only used by ReadAsynchron ...(which can be called multiple times in a row). But which actually looks wrong... Change-Id: I2e4914e6fed8ced383e430699dd462add9da8c08 diff --git a/include/tools/zcodec.hxx b/include/tools/zcodec.hxx index b93f796..c7424ab 100644 --- a/include/tools/zcodec.hxx +++ b/include/tools/zcodec.hxx @@ -54,7 +54,7 @@ class TOOLS_DLLPUBLIC ZCodec void* mpsC_Stream; void InitCompress(); - void InitDecompress(); + void InitDecompress(SvStream & inStream); void ImplWriteBack(); void UpdateCRC( sal_uInt8* pSource, long nDatSize ); diff --git a/tools/source/zcodec/zcodec.cxx b/tools/source/zcodec/zcodec.cxx index 1adbca8..47c97dc 100644 --- a/tools/source/zcodec/zcodec.cxx +++ b/tools/source/zcodec/zcodec.cxx @@ -115,11 +115,10 @@ long ZCodec::Compress( SvStream& rIStm, SvStream& rOStm ) long nOldTotal_In = PZSTREAM->total_in; assert(meState == STATE_INIT); - mpIStm = &rIStm; mpOStm = &rOStm; InitCompress(); mpInBuf = new sal_uInt8[ mnInBufSize ]; - while (( PZSTREAM->avail_in = mpIStm->Read( PZSTREAM->next_in = mpInBuf, mnInBufSize )) != 0 ) + while (( PZSTREAM->avail_in = rIStm.Read( PZSTREAM->next_in = mpInBuf, mnInBufSize )) != 0 ) { if ( PZSTREAM->avail_out == 0 ) ImplWriteBack(); @@ -139,9 +138,8 @@ long ZCodec::Decompress( SvStream& rIStm, SvStream& rOStm ) long nOldTotal_Out = PZSTREAM->total_out; assert(meState == STATE_INIT); - mpIStm = &rIStm; mpOStm = &rOStm; - InitDecompress(); + InitDecompress(rIStm); PZSTREAM->next_out = mpOutBuf = new sal_uInt8[ PZSTREAM->avail_out = mnOutBufSize ]; do { @@ -149,7 +147,7 @@ long ZCodec::Decompress( SvStream& rIStm, SvStream& rOStm ) if ( PZSTREAM->avail_in == 0 && mnInToRead ) { nInToRead = ( mnInBufSize > mnInToRead ) ? mnInToRead : mnInBufSize; - PZSTREAM->avail_in = mpIStm->Read( PZSTREAM->next_in = mpInBuf, nInToRead ); + PZSTREAM->avail_in = rIStm.Read( PZSTREAM->next_in = mpInBuf, nInToRead ); mnInToRead -= nInToRead; if ( mbUpdateCrc ) @@ -204,10 +202,9 @@ long ZCodec::Read( SvStream& rIStm, sal_uInt8* pData, sal_uIntPtr nSize ) if ( mbFinish ) return 0; // PZSTREAM->total_out; - mpIStm = &rIStm; if (meState == STATE_INIT) { - InitDecompress(); + InitDecompress(rIStm); } PZSTREAM->avail_out = nSize; PZSTREAM->next_out = pData; @@ -216,7 +213,7 @@ long ZCodec::Read( SvStream& rIStm, sal_uInt8* pData, sal_uIntPtr nSize ) if ( PZSTREAM->avail_in == 0 && mnInToRead ) { nInToRead = (mnInBufSize > mnInToRead) ? mnInToRead : mnInBufSize; - PZSTREAM->avail_in = mpIStm->Read ( + PZSTREAM->avail_in = rIStm.Read ( PZSTREAM->next_in = mpInBuf, nInToRead); mnInToRead -= nInToRead; @@ -251,8 +248,8 @@ long ZCodec::ReadAsynchron( SvStream& rIStm, sal_uInt8* pData, sal_uIntPtr nSize if (meState == STATE_INIT) { + InitDecompress(rIStm); mpIStm = &rIStm; - InitDecompress(); } PZSTREAM->avail_out = nSize; PZSTREAM->next_out = pData; @@ -340,7 +337,7 @@ void ZCodec::InitCompress() PZSTREAM->avail_out = mnOutBufSize; } -void ZCodec::InitDecompress() +void ZCodec::InitDecompress(SvStream & inStream) { assert(meState == STATE_INIT); meState = STATE_DECOMPRESS; @@ -349,45 +346,45 @@ void ZCodec::InitDecompress() sal_uInt8 n1, n2, j, nMethod, nFlags; for ( int i = 0; i < 2; i++ ) // gz - magic number { - mpIStm->ReadUChar( j ); + inStream.ReadUChar( j ); if ( j != gz_magic[ i ] ) mbStatus = false; } - mpIStm->ReadUChar( nMethod ); - mpIStm->ReadUChar( nFlags ); + inStream.ReadUChar( nMethod ); + inStream.ReadUChar( nFlags ); if ( nMethod != Z_DEFLATED ) mbStatus = false; if ( ( nFlags & GZ_RESERVED ) != 0 ) mbStatus = false; /* Discard time, xflags and OS code: */ - mpIStm->SeekRel( 6 ); + inStream.SeekRel( 6 ); /* skip the extra field */ if ( nFlags & GZ_EXTRA_FIELD ) { - mpIStm->ReadUChar( n1 ).ReadUChar( n2 ); - mpIStm->SeekRel( n1 + ( n2 << 8 ) ); + inStream.ReadUChar( n1 ).ReadUChar( n2 ); + inStream.SeekRel( n1 + ( n2 << 8 ) ); } /* skip the original file name */ if ( nFlags & GZ_ORIG_NAME) { do { - mpIStm->ReadUChar( j ); + inStream.ReadUChar( j ); } - while ( j && !mpIStm->IsEof() ); + while ( j && !inStream.IsEof() ); } /* skip the .gz file comment */ if ( nFlags & GZ_COMMENT ) { do { - mpIStm->ReadUChar( j ); + inStream.ReadUChar( j ); } - while ( j && !mpIStm->IsEof() ); + while ( j && !inStream.IsEof() ); } /* skip the header crc */ if ( nFlags & GZ_HEAD_CRC ) - mpIStm->SeekRel( 2 ); + inStream.SeekRel( 2 ); if ( mbStatus ) mbStatus = ( inflateInit2( PZSTREAM, -MAX_WBITS) != Z_OK ) ? false : true; } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits