[Libreoffice-commits] core.git: 2 commits - include/tools tools/source

Stephan Bergmann sbergman at redhat.com
Wed May 21 09:30:19 PDT 2014


 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 <sbergman at 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 <sbergman at 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;
     }


More information about the Libreoffice-commits mailing list