On Sat, 6 Sep 2008, Junio C Hamano wrote:
> Linus Torvalds  writes:

quoted text
>

> > The reason?

> >

> > Right now we depend on "avail_out" also making zlib understand to stop

> > looking at the input stream. Sad, but true - we don't know or care about

> > the compressed size of the object, only the uncompressed size. So in

> > unpack_compressed_entry(), we simply set the output length, and expect

> > zlib to stop when it's sufficient.

> >

> > Which it does - but the patch kind of violates that whole design.

> >

> > Now, it so happens that things seem to work, probably because the zlib

> > format does have enough synchronization in it to not try to continue past

> > the end _anyway_, but I think this makes the patch be of debatable value.

>

> I thought the fact we do check the status with Z_STREAM_END means that we

> do already expect and rely on zlib to know where the end of input stream

> is, and stop there (otherwise we say something fishy is going on and we

> error out), and it was part of the design, not just "so happens" and "has

> enough synch ... _anyway_".

>

> If input zlib stream were corrupted and it detected the end of stream too

> early, then check of "stream.total_out != size" would fail even though we

> would see "st == Z_STREAM_END".  If input stream were corrupted and it

> went past the end marker, we will read past the end and into some garbage

> that is the in-pack header of the next object representation, but zlib

> shouldn't go berserk even in that case, and would stop after filling the

> slop you allocated in the buffer --- we would detect the situation from

> stream.total_out != size and most likely st != Z_STREAM_END in such a

> case.

Unless I'm missing something, I think your analysis is right and

everything should be safe.
Nicolas

--

To unsubscribe from this list: send the line "unsubscribe git" in

the body of a message to majordomo@vger.kernel.org

More majordomo info at  http://vger.kernel.org/majordomo-info.html