[FFmpeg-devel] [PATCH 3/3] avcodec/mlpdec: Check quant_step_size against huff_lsbs

[Michael Niedermayer]

On Sun, May 21, 2017 at 01:42:18PM +0200, wm4 wrote:
> On Sat, 20 May 2017 23:01:04 +0200
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > This reorders the operations so as to avoid computations with the above arguments
> > before they have been initialized.
> > Fixes part of 1708/clusterfuzz-testcase-minimized-5035111957397504
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/mlpdec.c | 34 +++++++++++++++++++++++++---------
> >  1 file changed, 25 insertions(+), 9 deletions(-)
> > 
> > diff --git a/libavcodec/mlpdec.c b/libavcodec/mlpdec.c
> > index c0a23c5f0d..11be380d27 100644
> > --- a/libavcodec/mlpdec.c
> > +++ b/libavcodec/mlpdec.c
> > @@ -825,8 +825,6 @@ static int read_channel_params(MLPDecodeContext *m, unsigned int substr,
> >          return AVERROR_INVALIDDATA;
> >      }
> >  
> > -    cp->sign_huff_offset = calculate_sign_huff(m, substr, ch);
> > -
> >      return 0;
> >  }
> >  
> > @@ -838,7 +836,8 @@ static int read_decoding_params(MLPDecodeContext *m, GetBitContext *gbp,
> >  {
> >      SubStream *s = &m->substream[substr];
> >      unsigned int ch;
> > -    int ret;
> > +    int ret = 0;
> > +    unsigned recompute_sho = 0;
> >  
> >      if (s->param_presence_flags & PARAM_PRESENCE)
> >          if (get_bits1(gbp))
> > @@ -878,19 +877,36 @@ static int read_decoding_params(MLPDecodeContext *m, GetBitContext *gbp,
> >      if (s->param_presence_flags & PARAM_QUANTSTEP)
> >          if (get_bits1(gbp))
> >              for (ch = 0; ch <= s->max_channel; ch++) {
> > -                ChannelParams *cp = &s->channel_params[ch];
> > -
> >                  s->quant_step_size[ch] = get_bits(gbp, 4);
> >  
> > -                cp->sign_huff_offset = calculate_sign_huff(m, substr, ch);
> > +                recompute_sho |= 1<<ch;
> >              }
> >  
> >      for (ch = s->min_channel; ch <= s->max_channel; ch++)
> > -        if (get_bits1(gbp))
> > +        if (get_bits1(gbp)) {
> > +            recompute_sho |= 1<<ch;
> >              if ((ret = read_channel_params(m, substr, gbp, ch)) < 0)
> > -                return ret;
> > +                goto fail;
> > +        }
> >  
> > -    return 0;
> > +
> 
> 
> > +fail:
> > +    for (ch = 0; ch <= s->max_channel; ch++) {
> > +        if (recompute_sho & (1<<ch)) {
> > +            ChannelParams *cp = &s->channel_params[ch];
> > +
> > +            if (cp->codebook > 0 && cp->huff_lsbs < s->quant_step_size[ch]) {
> > +                if (ret >= 0) {
> > +                    av_log(m->avctx, AV_LOG_ERROR, "quant_step_size larger than huff_lsbs\n");
> > +                    ret = AVERROR_INVALIDDATA;
> > +                }
> > +                s->quant_step_size[ch] = 0;
> > +            }
> > +
> > +            cp->sign_huff_offset = calculate_sign_huff(m, substr, ch);
> > +        }
> > +    }
> > +    return ret;
> 
> What's all this stuff for?

As described in the commit message it
"Check quant_step_size against huff_lsbs"

both these are updated independant and conditionally, so the loop
checking them is seperate after both which is ugly.
If you have an idea how to do this cleaner ...

as it is in git the case checked for results in negative shift
exponents

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

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170522/b1bd4af3/attachment.sig>