[FFmpeg-devel] [PATCH 3/3] avcodec/x86/diracdsp: Fix incorrect src addressing in dequant_subband_32()

[Michael Niedermayer]

On Thu, Jan 30, 2020 at 05:14:18PM -0300, James Almer wrote:
> On 1/29/2020 6:55 PM, Michael Niedermayer wrote:
> > Fixes: Segfault (not reproducable with asm, which made this hard to debug)
> > Fixes: decoding errors
> > Fixes: 19854/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DIRAC_fuzzer-5729372837511168
> > 
> > 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/x86/diracdsp.asm | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/x86/diracdsp.asm b/libavcodec/x86/diracdsp.asm
> > index cc8a26fca5..a18bda113e 100644
> > --- a/libavcodec/x86/diracdsp.asm
> > +++ b/libavcodec/x86/diracdsp.asm
> > @@ -294,8 +294,9 @@ cglobal dequant_subband_32, 7, 7, 4, src, dst, stride, qf, qs, tot_v, tot_h
> >  
> >      add    srcq, mmsize
> >      add    dstq, mmsize
> > -    sub    tot_hd, 4
> > +    sub    tot_hq, 4
> 
> tot_h comes from stack, and on Windows x86_64 the higher 32 bits will be
> garbage. This should remain as tot_hd, so the sub instruction will
> implicitly clear said high bits (if the value could be negative, then
> you'd have to sign extend it instead).

well for this to work we need the tot_h value (which is not a multiple of 4
in the case of the bug) after the subtract to correct for that
non mod 4 == 0 case

is this fix below ok ?

diff --git a/libavcodec/x86/diracdsp.asm b/libavcodec/x86/diracdsp.asm
index a18bda113e..17145baf87 100644
--- a/libavcodec/x86/diracdsp.asm
+++ b/libavcodec/x86/diracdsp.asm
@@ -274,7 +274,7 @@ cglobal dequant_subband_32, 7, 7, 4, src, dst, stride, qf, qs, tot_v, tot_h
     movd   m3, qsd
     SPLATD m2
     SPLATD m3
-    mov    r4, tot_hq
+    mov    r4d, tot_hd
     mov    r3, dstq
 
     .loop_v:
     

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200130/0cadced2/attachment.sig>