diff mbox series

[FFmpeg-devel,3/3] avcodec/avpacket: Don't write into non-writable buffer

Message ID 20200212153127.1255-2-andreas.rheinhardt@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/3] avcodec/avpacket: Always treat dst in av_packet_ref as uninitialized
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt Feb. 12, 2020, 3:31 p.m. UTC
The data of an AVPacket may be a part of the data of an AVBufferRef;
Therefore av_grow_packet() doesn't reallocate if the available space in
the actual buffer is sufficient for the enlargement. But given that it
also zeroes the padding it also needs to make sure that the buffer is
actually writable; this commit implements this.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/avpacket.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Andreas Rheinhardt Feb. 12, 2020, 3:44 p.m. UTC | #1
On Wed, Feb 12, 2020 at 4:31 PM Andreas Rheinhardt <
andreas.rheinhardt@gmail.com> wrote:

> But given that it
> also zeroes the padding it also needs to make sure that the buffer is
> actually writable; this commit implements this.
>

av_shrink_packet has a similar issue and it seems unfixable given that it
is used with non-refcounted packets and doesn't return anything.

- Andreas
James Almer Feb. 12, 2020, 3:56 p.m. UTC | #2
On 2/12/2020 12:44 PM, Andreas Rheinhardt wrote:
> On Wed, Feb 12, 2020 at 4:31 PM Andreas Rheinhardt <
> andreas.rheinhardt@gmail.com> wrote:
> 
>> But given that it
>> also zeroes the padding it also needs to make sure that the buffer is
>> actually writable; this commit implements this.
>>
> 
> av_shrink_packet has a similar issue and it seems unfixable given that it
> is used with non-refcounted packets and doesn't return anything.
> 
> - Andreas

A solution could be introducing an av_packet_realloc/resize() function
that in practice implements both grow's and shrink's functionality, so
we can then deprecate the latter two (Good reason to do that as well,
seeing they don't use the av_packet_* namespace).
diff mbox series

Patch

diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 0d9ddeee07..a7b0b6bd5d 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -127,7 +127,8 @@  int av_grow_packet(AVPacket *pkt, int grow_by)
                 return AVERROR(ENOMEM);
         }
 
-        if (new_size + data_offset > pkt->buf->size) {
+        if (new_size + data_offset > pkt->buf->size ||
+            !av_buffer_is_writable(pkt->buf)) {
             int ret = av_buffer_realloc(&pkt->buf, new_size + data_offset);
             if (ret < 0) {
                 pkt->data = old_data;