Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#12228 closed Bug Report - Crash (fixed)

mythtranscode lossless mpeg seg faults in v0.28-pre-2014-gcc58fb0

Reported by: steve_g@… Owned by: JYA
Priority: minor Milestone: 0.28
Component: MythTV - Mythtranscode Version: Master Head
Severity: medium Keywords: mythtranscode
Cc: Ticket locked: no

Description

mythtranscode lossless mpeg seg faults in v0.28-pre-2014-gcc58fb0. In mpeg2fix.h, set_pkt it is using private code to copy the AVPacket which doesnt copy everything so later it segfaults on av_free_packet. Replace the code for the copy with av_copy_packet.

Patch Attached.

Attachments (2)

mpeg2segfault.patch (920 bytes) - added by steve_g@… 5 years ago.
gdb.txt (14.5 KB) - added by steve_g@… 5 years ago.
Backtrace

Download all attachments as: .zip

Change History (11)

Changed 5 years ago by steve_g@…

Attachment: mpeg2segfault.patch added

comment:1 Changed 5 years ago by JYA

Please provide backtrace of the crash.

using av_copy_packet as you did will leak as the data contained in the destination packet is never freed.

comment:2 Changed 5 years ago by stuartm

Status: newinfoneeded_new

The latest from master is v0.28-pre-2009-ge97f4f5, you're five commits ahead? What patches are you using?

comment:3 Changed 5 years ago by steve_g@…

Sorry about the version, it was taken from my patched copy. Got a clean copy from git this morning compiled and ran, problem is there and the version is v0.28-pre-2017-gccde729.

comment:4 Changed 5 years ago by steve_g@…

"using av_copy_packet as you did will leak as the data contained in the destination packet is never freed." The destination is "pkt", created with av_new_packet in the constructor, and freed with av_free_packet in the destructor of the class MPEG2frame, which is called from the destructor of MPEG2fixup. We might be losing the source packet though.

Im working on the backtrace.

Changed 5 years ago by steve_g@…

Attachment: gdb.txt added

Backtrace

comment:5 in reply to:  4 Changed 5 years ago by JYA

Replying to steve_g@…:

"using av_copy_packet as you did will leak as the data contained in the destination packet is never freed." The destination is "pkt", created with av_new_packet in the constructor, and freed with av_free_packet in the destructor of the class MPEG2frame, which is called from the destructor of MPEG2fixup. We might be losing the source packet though.

Im working on the backtrace.

This is not the issue.

In the current version, the content of the new packet (data) is copied into the old packet data. There's no new memory allocation.

av_packet_copy replaces the old packet data member with the new one. The old packet data now dangling. The code isn't equivalent. You must free the old packet before calling copy. Which means that now for every copy you need to free the packet, then copy (which will allocate a new one and perform the copy.

This is inefficient and not the way to do things

comment:6 Changed 5 years ago by paulh

Status: infoneeded_newnew

comment:7 Changed 5 years ago by Jean-Yves Avenard <jyavenard@…>

Resolution: fixed
Status: newclosed

In 3f70553d676774f808927dd8e9368f7fee874c09/mythtv:

mythtranscode: Prevent double-free

Not the best way to solve the problem, but it works...

Fixes #12228

comment:8 Changed 5 years ago by Stuart Morgan <smorgan@…>

In ad5f5890498f7ae1183359eaefd08e02544cc44a/mythtv:

Fix memory corruption issue in mythtranscode. Refs #12228

comment:9 Changed 5 years ago by steve_g@…

Thank you, this works for me.

Note: See TracTickets for help on using tickets.