Opened 18 years ago
Closed 18 years ago
Last modified 18 years ago
#1392 closed defect (fixed)
invalid read in PESPacket::PESPacket()
Reported by: | Owned by: | danielk | |
---|---|---|---|
Priority: | minor | Milestone: | 0.20 |
Component: | dvb | Version: | head |
Severity: | low | Keywords: | |
Cc: | Ticket locked: | no |
Description
valgrind complains of invalid read in PESPacket::PESPacket() called from P[A|M]T::CreateBlank?()
valgring output attached
Attachments (3)
Change History (7)
Changed 18 years ago by
Attachment: | PMTPAT.valgrind added |
---|
comment:1 Changed 18 years ago by
Changed 18 years ago by
Attachment: | pmtpat_valgrind.txt added |
---|
Changed 18 years ago by
Attachment: | pespacket_valgrind_silencing.patch added |
---|
comment:2 Changed 18 years ago by
Attached patch fixes the valgrind errors.
I'm not totaly sure of the semantics of _pesdataSize. But I think the patch is correct.
comment:3 Changed 18 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
fixed in [9328]
thanks. the 0x00 in front of the sync byte looked suspicious, but I thought it was by design.
comment:4 Changed 18 years ago by
Milestone: | → 0.20 |
---|---|
Version: | → head |
That 0x00 is for the start of field pointer, but since I moved the offset of the PES packets to match the DVB drivers a few weeks ago it should now be considered part of the TS packet rather than the PES packet so I moved it. This also makes more sense in the case of multiple PSIP sections per TS packet, which is allowed by the DVB standard (but not by the ATSC for which this parser was originally written.)
The actual fix in the patch was in the length setting in those CreateBlank? functions. We set the length so that exactly 188 bytes of the 188 byte TS Packet are copied just before the packet copy, and then set it to the proper length right afterward. This means we copy just the right number of bytes, but the length is still set properly by the end of the function.
updated valgrind summary,
Now only a of by one error.