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

[Aki Tuomi]

> On July 5, 2017 at 9:24 PM Anton Yuzhaninov <citrin at citrin.ru> 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/6a1110d8757bb72fd90f4fe0857fd3aeaf8920ff
> >>
> >> "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