GNOME Bugzilla – Bug 102719
[PATCH] gst_mpeg_parse_parse_packhead reads outside the buffer
Last modified: 2004-12-22 21:47:04 UTC
valgrind is amazing. three cheers for valgrind.
Created attachment 13394 [details] [review] re-code logic to avoid reading outside the buffer
Joshua, Thanks for that patch. a few remarks: - the GUINT-macro's are very useful for endianness, so not using them will probably cause us a lot of problems in that area. So I think we should keep using them. - can you say how it can happen that the 4th byte is out of range ? I suspect valgrind put you on the track, but did you also find out why this could be a problem ? - isn't there a possibility for a workaround to check this, and then use an endian-correct way to get at the data ? An alternative might be to allocate one byte more by default for the buffer or something. - please don't use // :)
> - the GUINT-macro's are very useful for endianness, so not using them > will probably cause us a lot of problems in that area. So I think we > should keep using them. That's nonsense. When reading single bytes then there is no such thing as endianness. Bytes in the stream are always ordered the same way. So my patch is also an optimization (unless the compiler optimizes out the byte-swapping). So if this is the reason you've been waiting to commit my patch .. please commit! > can you say how it can happen that the 4th byte is out of range ? I > suspect valgrind put you on the track, but did you also find out why > this could be a problem ? Valgrind found it. It just happens that the 4th byte is outside the buffer occationally when indexing (or playing, i suppose) a large media file. Try scanning through a VCD or DVD and you'll hit the crash. i don't have any deeper explanation because i don't understand most of the MPEG parsing code. i'm just making a technical fix. > please don't use // Yah, sorry. It's hard to break good habits. ;-)
ok, committed