[FFmpeg-devel] [PATCH] avformat/av1: Initialize padding in ff_isom_write_av1c

[James Almer]

On 4/8/2019 10:53 AM, Jeremy Dorfman via ffmpeg-devel wrote:
> Judging by the code, I think flush_put_bits only pads out to a byte
> boundary rather than the end of the buffer. By my read it will run the body
> of the while loop 3 times in this case, for bit_left = 8, 16, 24, so only 3
> bytes are filled in header.
> 
> I'd expect this to only affect the actual output in uncommon cases. It
> requires the reuse of a stack with non-zero values at the location of
> uint8_t header[4]. In the common case of running FFmpeg, it looks like this
> is called from the main thread so it should typically be zeroed by the OS.
> MemorySanitizer is tagging that region of the stack as uninitialized, so I
> don't think this is a false positive -- it doesn't appear to ever be
> intentionally written.
> 
> Thanks,
> -Jeremy

Alright, thanks for the explanation. Pushed.

> 
> On Mon, Apr 8, 2019 at 9:33 AM James Almer <jamrial at gmail.com> wrote:
> 
>> On 4/8/2019 9:14 AM, Jeremy Dorfman via ffmpeg-devel wrote:
>>> Otherwise, AV1 encodes with FFmpeg trigger use-of-uninitialized-value
>>> warnings under MemorySanitizer, and the output buffer potentially
>>> changes from run to run.
>>> ---
>>>  libavformat/av1.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/libavformat/av1.c b/libavformat/av1.c
>>> index a0aad436a6..5fde8df97e 100644
>>> --- a/libavformat/av1.c
>>> +++ b/libavformat/av1.c
>>> @@ -372,6 +372,7 @@ int ff_isom_write_av1c(AVIOContext *pb, const
>> uint8_t *buf, int size)
>>>      put_bits(&pbc, 1, seq_params.chroma_subsampling_x);
>>>      put_bits(&pbc, 1, seq_params.chroma_subsampling_y);
>>>      put_bits(&pbc, 2, seq_params.chroma_sample_position);
>>> +    put_bits(&pbc, 8, 0); // padding
>>
>> Shouldn't flush_put_bits() below do this? The doxy says "Pad the end of
>> the output stream with zeros".
>> In any case, i just tried remuxing a single file several times, and in
>> all cases the hash of the output file was the same, so it could be a
>> false positive from MemorySanitizer.
>>
>> Patch is fine either way, but i'd like to know why you're getting those
>> warnings.
>>
>>>      flush_put_bits(&pbc);
>>>
>>>      avio_write(pb, header, sizeof(header));
>>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>