Opened 6 years ago
Last modified 5 years ago
#7484 new defect
av_packet_ref(): Allocates array on zero src size
| Reported by: | Gavin | Owned by: | |
|---|---|---|---|
| Priority: | normal | Component: | avcodec |
| Version: | git-master | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Reproduced by developer: | no | |
| Analyzed by developer: | no |
Description
Discovered a bug on a corner-case when exercising some unit tests I had written. The code involved pertains to function:
int av_packet_ref(AVPacket *dst, const AVPacket *src).
I'll jump right into the code example:
Code highlighting:
#include <cassert> extern "C" { #include <libavcodec/avcodec.h> } int main(int argc, char *argv[]) { AVPacket pkt{}; av_init_packet(&pkt); assert(pkt.size == 0); // OK; assert(pkt.data == nullptr); // OK AVPacket pkt2{}; av_init_packet(&pkt2); assert(pkt2.size == 0); // OK; assert(pkt2.data == nullptr); // OK assert(0 == av_packet_ref(&pkt2, &pkt)); // OK? Discuss. assert(pkt.size == 0); // OK; assert(pkt.data == nullptr); // OK assert(pkt2.size == 0); // OK; assert(pkt2.data == nullptr); // ASSERTS: Woah! Not good! return 0; }
So av_packet_ref(...) func is called with a Src and Dst pkt whose size is both 0 and data is both NULL, yet after the function is called the data on the dst is non-NULL.
Code sample built using the following command:
g++ main.cpp -lavcodec -o test
Ubuntu 18.04
GCC 8.2
FFmpeg v4.0.2
Change History (2)
comment:1 by , 5 years ago
comment:2 by , 5 years ago
| Version: | → git-master |
|---|
Why do you believe the current behaviour is wrong? Is it documented differently?
It seems that at least ff_read_packet() expects the current behaviour when doing err = av_packet_make_refcounted(pkt);
If you want fix the failing fate tests, you can replace return AVERROR(EINVAL) with abort() to find the offending calls.
I took a stab at this over the end-of-year break. I think the best way to solve this is to disable allocation of an AVPacket with size of 0:
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c index e160ad3033..df4cafc6da 100644 --- a/libavcodec/avpacket.c +++ b/libavcodec/avpacket.c @@ -71,7 +71,7 @@ void av_packet_free(AVPacket **pkt) static int packet_alloc(AVBufferRef **buf, int size) { int ret; - if (size < 0 || size >= INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE) + if (size <= 0 || size >= INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE) return AVERROR(EINVAL); ret = av_buffer_realloc(buf, size + AV_INPUT_BUFFER_PADDING_SIZE);I applied the above patch to latest.
As per the development guidelines, I tried out the fate suite of tests, but I notice that the
acodec-flactest fails which could be bad. I tried to follow the make file, but I struggled to quite understand how fate works in order to debug.