[FFmpeg-devel] [PATCH] golomb: check log validity before shifting

Michael Niedermayer michaelni at gmx.at
Fri Jan 18 15:39:29 CET 2013


On Tue, Jan 15, 2013 at 12:28:43AM +0100, Michael Niedermayer wrote:
> On Mon, Jan 14, 2013 at 10:43:17PM +0000, Paul B Mahol wrote:
> > On 1/14/13, Michael Niedermayer <michaelni at gmx.at> wrote:
> > > On Mon, Jan 14, 2013 at 11:58:13AM +0000, Paul B Mahol wrote:
> > >> On 1/12/13, Michael Niedermayer <michaelni at gmx.at> wrote:
> > >> > Fixes invalid right shift in fate-cavs
> > >> >
> > >> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > >> > ---
> > >> >  libavcodec/golomb.h |    8 ++++++--
> > >> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > >> >
> > >> > diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> > >> > index 0629c78..e3a35e9 100644
> > >> > --- a/libavcodec/golomb.h
> > >> > +++ b/libavcodec/golomb.h
> > >> > @@ -66,10 +66,14 @@ static inline int get_ue_golomb(GetBitContext *gb){
> > >> >          return ff_ue_golomb_vlc_code[buf];
> > >> >      }else{
> > >> >          log= 2*av_log2(buf) - 31;
> > >> > -        buf>>= log;
> > >> > -        buf--;
> > >> >          LAST_SKIP_BITS(re, gb, 32 - log);
> > >> >          CLOSE_READER(re, gb);
> > >> > +        if (log < 0) {
> > >> > +            av_log(0, AV_LOG_ERROR, "Invalid UE golomb code\n");
> > >> > +            return AVERROR_INVALIDDATA;
> > >> > +        }
> > >> > +        buf>>= log;
> > >> > +        buf--;
> > >> >
> > >> >          return buf;
> > >> >      }
> > >> > --
> > >> > 1.7.9.5
> > >>
> > >> I'm not sure about return code, most code that calls it never check
> > >> value.
> > >
> > > thats true, do you see a reason this code would be worse than -1 or
> > > 0 ? (0 would not allow the condition to be detected ...)
> > 
> > Whatever that really fix bugs and not just fixes one and leave others flying.
> 
> It should fix the IOC failure and makes the invalid cases more
> vissible. It doesnt make callers check the return code. Later may
> in some cases have speedloss implications if it where done
> 
> ill wait a day and if i hear no objections from anyone then ill commit

made it conditional on a numeric anomaly checker being enabled
and applied
maybe the check should always be done but that can be added later
ATM the only known issue is IOC failure and adding code may have
speed implications ...

[...]

-- 
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: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130118/ba87e89c/attachment.asc>


More information about the ffmpeg-devel mailing list