Nicolas Pitre <nico@xxxxxxx> writes: > On Sat, 6 Sep 2008, Junio C Hamano wrote: > >> Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes: >> ... >> > 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. I obviously agree with you but what I forgot to mention in the above is that we also make sure stream.avail_in is set not to overrun the end of the current pack window (or the entire loose object data that is mmapped). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
