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

[Michael Niedermayer]

On Thu, Jan 30, 2020 at 10:11:05PM +0100, Michael Niedermayer wrote:
> 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 ?

i succeded to reproduce this and the bugfix below on a netbsd VM, which
unexpectedly seems to be affected by this too

on mingw32/64 no issue is reproducable unless i hack the asm to make one which 
then gets fixed with the code.

i dont want to leave the regression i caused in the tree and go to bed 
so i guess ill apply this. But this issue is cursed and my asm is a bit
rusty so i have a bit an uneasy feeling.
If this fix doesnt fix it for real, dont hesitate to revert it and the
buggy fix before

Thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Any man who breaks a law that conscience tells him is unjust and willingly 
accepts the penalty by staying in jail in order to arouse the conscience of 
the community on the injustice of the law is at that moment expressing the 
very highest respect for law. - Martin Luther King Jr
-------------- 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/20200131/58109548/attachment.sig>