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

[Jeremy Dorfman]

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

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".