[FFmpeg-devel] [PATCH] avcodec/v210dec: Fix alignment check for AVX2

[Michael Niedermayer]

On Sat, May 18, 2019 at 01:05:01PM +0200, Reimar Döffinger wrote:
> On 18.05.2019, at 12:15, Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > On Sat, May 18, 2019 at 12:02:55PM +0200, James Darnley wrote:
> >> On 2019-05-18 09:39, Michael Niedermayer wrote:
> >>> Fixes: "null pointer dereference"
> >>> Fixes: 14551/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_V210_fuzzer-5088609952071680
> >>> 
> >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> >>> ---
> >>> libavcodec/v210dec.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>> 
> >>> diff --git a/libavcodec/v210dec.c b/libavcodec/v210dec.c
> >>> index bc1e1d34ff..5a33d8c089 100644
> >>> --- a/libavcodec/v210dec.c
> >>> +++ b/libavcodec/v210dec.c
> >>> @@ -104,7 +104,7 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
> >>>         && avpkt->size - 64 >= stride * avctx->height)
> >>>         psrc += 64;
> >>> 
> >>> -    aligned_input = !((uintptr_t)psrc & 0xf) && !(stride & 0xf);
> >>> +    aligned_input = !((uintptr_t)psrc & 0x1f) && !(stride & 0x1f);
> >>>     if (aligned_input != s->aligned_input) {
> >>>         s->aligned_input = aligned_input;
> >>>         ff_v210dec_init(s);
> >>> 
> >> 
> >> Ah yes, that'll be needed after the recent addition of avx2.  LGTM and
> >> sorry.
> >> 
> > 
> >> I object to the commit message though because it isn't a "null pointer
> >> dereference" but if that is the error as reported by the tool then keep
> >> it as is.
> > 
> > yes, the tool(s) say things like "Null-dereference READ", "SEGV on unknown address 0x000000000000"
> 
> Is that a tool bug or are we missing something that goes on here besides the alignment?
> If it's a tool bug can we try to get it fixed?
> This has a big risk of wasting time if they print straight-up wrong information...

I do not know for 100% sure. I actually have to admit that i tested on a 
pre AVX2 CPU (sandybridge). I confirmed the bug and fix on a AVX2 box but
the ossfuzz stuff is not setup on that box.
What ossfuzz on the web interface says is that theres a null pointer 
dereference in ff_v210_planar_unpack_aligned_avx2. And no anomaly is
reproducable on sandybridge. 
The tools seem to lack support for providing usefull information about asm
code. Theres for example no line numbers, no register values, ...
Ill try to report this issue in case this patch fixes it (and thus proofs
that the alignment fix indeed matches the null pointer report.

Thanks

PS: yes i know i should upgrade that sandybridge box, i had planed to but
theres always something that causes me to move it down on my todo list ...


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190518/20955595/attachment.sig>