bugGNU Octave - Bugs: bug #60450, load fails to read MAT file

 
 

bug #60450: load fails to read MAT file

Submitter:  John W. Eaton <jwe>
Submitted:  Sat 24 Apr 2021 01:59:42 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Works For Me Assigned to:  None
Originator Name:  jwe Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 28 Apr 2021 10:37:32 PM UTC, comment #16: 

Turns out it isn't easy to replace zlib. I assume it's used at boot time to uncompress the kernel; with some other zlib my system stalled or had a kernel panic very early during boot. Maybe there are safety measures to ensure a particular zlib version is tied to the boot process? (wouldn't be a bad idea from a security perspective)
No appetite here to experiment further :-)

Anyway I've been pondering for some time to change distro anyway - Mageia 6 to 8 had/have issues with my (older) video HW (tried several other distro's that just work) + it used to have some older libraries which has bitten me before when building dev Octave. Maybe this zlib thing should be considered the tipping point.

So in summary I'd say: yes, just close this bug report.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 28 Apr 2021 10:01:34 PM UTC, comment #15: 

@Philip: Did replacing lib64zlib work?  Can we close this report?

Rik <rik5>
Group administrator
Mon 26 Apr 2021 01:55:00 PM UTC, comment #14: 

If that's needed for just one distro I wouldn't bother. mxe-octave has zlib 1.2.11 including uncompress2 so for Windows builds there's no issue either.

I'll try to replace lib64zlib in my Mageia installation.

Philip Nienhuis <philipnienhuis>
Group Member
Mon 26 Apr 2021 12:35:12 PM UTC, comment #13: 

The uncompress2 function is just the following.


/* ===========================================================================
     Decompresses the source buffer into the destination buffer.  *sourceLen is
   the byte length of the source buffer. Upon entry, *destLen is the total size
   of the destination buffer, which must be large enough to hold the entire
   uncompressed data. (The size of the uncompressed data must have been saved
   previously by the compressor and transmitted to the decompressor by some
   mechanism outside the scope of this compression library.) Upon exit,
   *destLen is the size of the decompressed data and *sourceLen is the number
   of source bytes consumed. Upon return, source + *sourceLen points to the
   first unused input byte.

     uncompress returns Z_OK if success, Z_MEM_ERROR if there was not enough
   memory, Z_BUF_ERROR if there was not enough room in the output buffer, or
   Z_DATA_ERROR if the input data was corrupted, including if the input data is
   an incomplete zlib stream.
*/
int ZEXPORT uncompress2 (dest, destLen, source, sourceLen)
    Bytef *dest;
    uLongf *destLen;
    const Bytef *source;
    uLong *sourceLen;
{
    z_stream stream;
    int err;
    const uInt max = (uInt)-1;
    uLong len, left;
    Byte buf[1];    /* for detection of incomplete stream when *destLen == 0 */

    len = *sourceLen;
    if (*destLen) {
        left = *destLen;
        *destLen = 0;
    }
    else {
        left = 1;
        dest = buf;
    }

    stream.next_in = (z_const Bytef *)source;
    stream.avail_in = 0;
    stream.zalloc = (alloc_func)0;
    stream.zfree = (free_func)0;
    stream.opaque = (voidpf)0;

    err = inflateInit(&stream);
    if (err != Z_OK) return err;

    stream.next_out = dest;
    stream.avail_out = 0;

    do {
        if (stream.avail_out == 0) {
            stream.avail_out = left > (uLong)max ? max : (uInt)left;
            left -= stream.avail_out;
        }
        if (stream.avail_in == 0) {
            stream.avail_in = len > (uLong)max ? max : (uInt)len;
            len -= stream.avail_in;
        }
        err = inflate(&stream, Z_NO_FLUSH);
    } while (err == Z_OK);

    *sourceLen -= len + stream.avail_in;
    if (dest != buf)
        *destLen = stream.total_out;
    else if (stream.total_out && err == Z_BUF_ERROR)
        left = 1;

    inflateEnd(&stream);
    return err == Z_STREAM_END ? Z_OK :
           err == Z_NEED_DICT ? Z_DATA_ERROR  :
           err == Z_BUF_ERROR && left + stream.avail_out ? Z_DATA_ERROR :
           err;
}


It only requires a few typedefs and the inflate function.  But the logic is apparently wrong because the Z_BUF_ERROR or Z_DATA_ERROR status can escape the function when the uncompress action appears to actually be correct and the buffer limit is not actually exceeded.  So if someone could figure out how to use inflate correctly, we could just avoid the missing function problem.

John W. Eaton <jwe>
Group administrator
Mon 26 Apr 2021 07:49:54 AM UTC, comment #12: 

I've also searched in vain on the Mageia site for a motive, but it looks a bit like it is a forgotten issue there. So I've entered a bug report here:
https://bugs.mageia.org/show_bug.cgi?id=28839

In order to crossbuild for Windows a dist archive is needed but that won't happen unless this issue is fixed or avoided. So for now I'll back out jwe's cset, rather than workaround it by e.g., building my own zlib or so.

Philip Nienhuis <philipnienhuis>
Group Member
Mon 26 Apr 2021 07:10:47 AM UTC, comment #11: 

Thanks Dmitri for those forensics.

Maybe I could revert somehow to zlib-1.10.xx. The change is from very early 2017 so quite a few recent Mageia versions have been affected.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 25 Apr 2021 10:53:41 PM UTC, comment #10: 

It looks like it was removed by David Walser <luigiwalser@yahoo.com> of mageis security team.
I could not find any explanation, perhaps you can ask him why.

http://sophie.zarb.org/rpms/808f21768738fc8ffb1a1332c3cb7dbe/changelog

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Sun 25 Apr 2021 10:40:33 PM UTC, comment #9: 

It is funny:

diff zlib.h /usr/include/zlib.h
1283a1284,1290

> ZEXTERN int ZEXPORT uncompress2 OF((Bytef *dest,   uLongf *destLen,
>                                     const Bytef *source, uLong *sourceLen));
> /*
>      Same as uncompress, except that sourceLen is a pointer, where the
>    length of the source is *sourceLen.  On return, *sourceLen is the number of
>    source bytes consumed.
> */


Dmitri A. Sergatskov <dasergatskov>
Sun 25 Apr 2021 10:34:42 PM UTC, comment #8: 

On Mageia 8 it apparently doesn't (grep doesn't see it), neither uncompress(2) nor uncompress2. It does mention compress2 though. Fascinating ... or my goggles need an upgrade?

I've attached it for reference.

(file #51320)

Philip Nienhuis <philipnienhuis>
Group Member
Sun 25 Apr 2021 07:53:26 PM UTC, comment #7: 

This compiles fine for me with Kubuntu 18.04 distribution.

You might look at your <zlib.h> file.  On Kubuntu, this is in /usr/include.  This should have the prototype for uncompress2().

Rik <rik5>
Group administrator
Sun 25 Apr 2021 10:11:58 AM UTC, comment #6: 

BTW, reading comments #3 and #4, would those relate to bug #55427? (a Windows bug admittedly)

Philip Nienhuis <philipnienhuis>
Group Member
Sun 25 Apr 2021 09:54:57 AM UTC, comment #5: 

With that change from comment #2, I get the following on Mageia 8 Linux:

:
  CXX      libinterp/corefcn/libcorefcn_la-ls-mat-ascii.lo
  CXX      libinterp/corefcn/libcorefcn_la-ls-mat4.lo
  CXX      libinterp/corefcn/libcorefcn_la-ls-mat5.lo
  CXX      libinterp/corefcn/libcorefcn_la-ls-oct-binary.lo
  CXX      libinterp/corefcn/libcorefcn_la-ls-oct-text.lo
../dev/libinterp/corefcn/ls-mat5.cc: In function ‘std::string read_mat5_binary_element(std::istream&, const string&, bool, bool&, octave_value&)’:
../dev/libinterp/corefcn/ls-mat5.cc:534:11: error: ‘uncompress2’ was not declared in this scope; did you mean ‘uncompress’?
  534 |       if (uncompress2 (reinterpret_cast<Bytef *> (tmp), &destLen,
      |           ^~~~~~~~~~~
      |           uncompress
../dev/libinterp/corefcn/ls-mat5.cc:549:17: error: ‘uncompress2’ was not declared in this scope; did you mean ‘uncompress’?
  549 |       int err = uncompress2 (reinterpret_cast<Bytef *>
      |                 ^~~~~~~~~~~
      |                 uncompress
make[2]: *** [Makefile:19518: libinterp/corefcn/libcorefcn_la-ls-mat5.lo] Error 1
make[2]: *** Waiting for unfinished jobs....


FWIW, on my system I have lib64zlib-devel 1.2.11 9.mga8

Philip Nienhuis <philipnienhuis>
Group Member
Sat 24 Apr 2021 01:19:15 PM UTC, comment #4: 

Yes, we could probably switch to using inflate in a loop and do all the error checking ourselves, but we already know precisely the expected output size and completely uncompressing a buffer in one call is what uncompress{,2} from zlib are already supposed to do, so it would be great if those functions got it right and just reliably uncompressed the data block.

John W. Eaton <jwe>
Group administrator
Sat 24 Apr 2021 12:44:08 PM UTC, comment #3: 

I really do not understand this, but I saw in the FAQ:


http://zlib.net/zlib_faq.html#faq05

deflate() or inflate() returns Z_BUF_ERROR.
Before making the call, make sure that avail_in and avail_out are not zero. When setting the parameter flush equal to Z_FINISH, also make sure that avail_out is big enough to allow processing all pending input. Note that a Z_BUF_ERROR is not fatal--another call to deflate() or inflate() can be made with more input or output space. A Z_BUF_ERROR may in fact be unavoidable depending on how the functions are used, since it is not possible to tell whether or not there is more output pending when strm.avail_out returns with zero. See http://zlib.net/zlib_how.html for a heavily annotated example.


Just fyi.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Sat 24 Apr 2021 12:34:19 PM UTC, comment #2: 

For now, I pushed the following changeset:

http://hg.savannah.gnu.org/hgweb/octave/rev/87eff21d2609

John W. Eaton <jwe>
Group administrator
Sat 24 Apr 2021 02:12:22 AM UTC, comment #1: 

I'm also attaching a tar.gz file that includes the source for a  standalone program and two data files (the compressed data from the original problem MAT file and the expected uncompressed data).  The program reads both files, uncompresses the compressed data into a separate buffer, and then compares it with the expected uncompressed data.  For me, the behavior is the same as for Octave, so solving the problem with the simpler program will probably allow us to easily fix it in Octave as well.

Here is what I see:


$ tar zxf uncompress2-test.tar.gz
$ cd uncompress2-test
$ g++ -O2 uncompress2-test.cc -lz
$ ./a.out
status = -5 NOT OK!
output buffer size = 3506176 (buffer is sized exactly as needed)
result size = 3506176 OK
uncompressed data compares EQUAL to original data


The status == -5 is Z_BUF_ERROR, which indicates that the output buffer is too small.  But it is not, and even if I enlarge it, there is still an error, though I believe is it Z_DATA_ERROR, not Z_BUF_ERROR.

If someone would like to report this problem upstream, you may use this program for the report.  But I don't have much hope for a resolution as development on zlib appears to be stalled.

John W. Eaton <jwe>
Group administrator
Sat 24 Apr 2021 01:59:42 AM UTC, original submission:  

Attempting to load the attached MAT file (created with Matlab) fails with the following error:


octave:1> load Aj_23_640.mat
error: load: error uncompressing data element (buf error from zlib)


After much puzzling over the reason for the error, I've concluded that the compressed data block in the file is uncompressed correctly by zlib, but the uncompress function that we are using to do that is returning Z_BUF_ERROR when I think it should be Z_OK.

I'm using zlib 1.2.11.dfsg-2 on a Debian system.

I have a workaround (ignore the Z_BUF_ERROR condition if the compressed data is completely processed and the uncompressed data is the expected size) but that seems klugey.  Maybe someone who understands zlib better than me can comment?

The code that performs the uncompression is here:  http://hg.savannah.gnu.org/hgweb/octave/file/33556123b892/libinterp/corefcn/ls-mat5.cc#l518

The code that does the uncompress step is here:

John W. Eaton <jwe>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #51320:  zlib.h added by philipnienhuis (94KiB - text/x-chdr)
file #51313:  Aj_23_640.mat added by jwe (714KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by philipnienhuis
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by jwe (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-04-29 rik5 StatusIn Progress Works For Me
        Open/ClosedOpen Closed
    2021-04-25 philipnienhuis Attached File- Added zlib.h, #51320
        Carbon-Copy- Added philipnienhuis
    2021-04-24 jwe StatusNone In Progress
    2021-04-24 jwe Attached File- Added Aj_23_640.mat, #51313

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code