Re: gzip compressed mbox: Panic: file istream-zlib.c: line 421 (i_stream_zlib_seek): assertion failed: (ret == -1)

On July 5, 2017 at 9:24 PM Anton Yuzhaninov wrote:

On 07/05/17 13:25, Aki Tuomi wrote:

...

...

I use mbox compressed by gzip as read-only folder (using zlib plugin).

I have errors in logs: Panic: file istream-zlib.c: line 416 (i_stream_zlib_seek): assertion failed: (ret == -1) ... The bug appeared in 2.2.26 and still can be reproduced (in 2.2.31)

Using 'git bisect' was not productive (a lot of build errors), but I've found that reverting this commit: https://github.com/dovecot/core/commit/6a1110d8757bb72fd90f4fe0857fd3aeaf892...

"fixes" problem for me, so I suppose the bug was introduced in this commit.

...

If you only revert the bit here

- } while (i_stream_read(&stream->istream) > 0); + } while ((ret = i_stream_read(&stream->istream)) > 0);

in src/lib-compression/istream-zlib.c

does it still fix the issue?

With this change applied to fresh origin/master-2.2 I see no error: --- a/src/lib-compression/istream-zlib.c +++ b/src/lib-compression/istream-zlib.c @@ -417,7 +417,7 @@ i_stream_zlib_seek(struct istream_private *stream, uoff_t v_offset, bool mark) }

i_stream_skip(&stream->istream, avail); - } while ((ret = i_stream_read(&stream->istream)) > 0); + } while (i_stream_read(&stream->istream) > 0); i_assert(ret == -1);

if (stream->istream.v_offset != v_offset) {

But if ret is not assigned, then assert below is useless.

May be better fix will be: index f7354d83d..06389362a 100644 --- a/src/lib-compression/istream-zlib.c +++ b/src/lib-compression/istream-zlib.c @@ -404,7 +404,7 @@ i_stream_zlib_seek(struct istream_private *stream, uoff_t v_offset, bool mark) stream->pos = stream->skip; } else { /* read and cache forward */ - ssize_t ret = -1; + ssize_t ret;

do { size_t avail = stream->pos - stream->skip; @@ -413,6 +413,7 @@ i_stream_zlib_seek(struct istream_private *stream, uoff_t v_offset, bool mark) i_stream_skip(&stream->istream, v_offset - stream->istream.v_offset); + ret = -1; break; }

It also works for me.

Also similar code can be found in files for other compression formats.

Question is though, is the original code returning ret = -2? Is it possible for you find out? Aki