| Message ID | 20200720061545.18854-2-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | 78f21cab188a094d42520bcad9686c3b5afa844b |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avformat/mxfdec: Fix memleak when adding element to array fails | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
mån 2020-07-20 klockan 08:15 +0200 skrev Andreas Rheinhardt: > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -2714,6 +2714,7 @@ static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = { > > static int mxf_metadataset_init(MXFMetadataSet *ctx, enum MXFMetadataSetType type) > { > + ctx->type = type; > switch (type){ > case MultipleDescriptor: > case Descriptor: > @@ -2734,7 +2735,8 @@ static int mxf_read_local_tags(MXFContext *mxf, KLVPacket *klv, MXFMetadataReadF > > if (!ctx) > return AVERROR(ENOMEM); > - mxf_metadataset_init(ctx, type); > + if (ctx_size) > + mxf_metadataset_init(ctx, type); > while (avio_tell(pb) + 4 < klv_end && !avio_feof(pb)) { > int ret; > int tag = avio_rb16(pb); > @@ -2770,7 +2772,6 @@ static int mxf_read_local_tags(MXFContext *mxf, KLVPacket *klv, MXFMetadataReadF > * it extending past the end of the KLV though (zzuf5.mxf). */ > if (avio_tell(pb) > klv_end) { > if (ctx_size) { > - ctx->type = type; > mxf_free_metadataset(&ctx, 1); > } > > @@ -2781,7 +2782,6 @@ static int mxf_read_local_tags(MXFContext *mxf, KLVPacket *klv, MXFMetadataReadF > } else if (avio_tell(pb) <= next) /* only seek forward, else this can loop for a long time */ > avio_seek(pb, next, SEEK_SET); > } > - if (ctx_size) ctx->type = type; Looks OK as far as I can tell. It's been a while since I dug into mxfdec's type system. /Tomas
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c index 08ad92cc0c..3016885e75 100644 --- a/libavformat/mxfdec.c +++ b/libavformat/mxfdec.c @@ -2714,6 +2714,7 @@ static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = { static int mxf_metadataset_init(MXFMetadataSet *ctx, enum MXFMetadataSetType type) { + ctx->type = type; switch (type){ case MultipleDescriptor: case Descriptor: @@ -2734,7 +2735,8 @@ static int mxf_read_local_tags(MXFContext *mxf, KLVPacket *klv, MXFMetadataReadF if (!ctx) return AVERROR(ENOMEM); - mxf_metadataset_init(ctx, type); + if (ctx_size) + mxf_metadataset_init(ctx, type); while (avio_tell(pb) + 4 < klv_end && !avio_feof(pb)) { int ret; int tag = avio_rb16(pb); @@ -2770,7 +2772,6 @@ static int mxf_read_local_tags(MXFContext *mxf, KLVPacket *klv, MXFMetadataReadF * it extending past the end of the KLV though (zzuf5.mxf). */ if (avio_tell(pb) > klv_end) { if (ctx_size) { - ctx->type = type; mxf_free_metadataset(&ctx, 1); } @@ -2781,7 +2782,6 @@ static int mxf_read_local_tags(MXFContext *mxf, KLVPacket *klv, MXFMetadataReadF } else if (avio_tell(pb) <= next) /* only seek forward, else this can loop for a long time */ avio_seek(pb, next, SEEK_SET); } - if (ctx_size) ctx->type = type; return ctx_size ? mxf_add_metadata_set(mxf, &ctx) : 0; }
The MXF demuxer uses an array of pointers to different structures of metadata (all containing a common initial sequence containing a type field to distinguish them) and some of these structures contain pointers to separately allocated subelements. If an error happens while reading and creating the tags, the semi-finished new tag is freed using the function to free these tags. But this function doesn't free the already allocated subelements, because the type has not been set yet. This commit changes this. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavformat/mxfdec.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)