[FFmpeg-devel] [PATCH 1/2] avcodec/rangecoder: Do not increase the pointer beyond the buffer

[Michael Niedermayer]

On Wed, Aug 16, 2017 at 06:27:19PM +0200, Michael Niedermayer wrote:
> On Mon, Aug 14, 2017 at 11:30:06PM -0300, James Almer wrote:
> > On 8/14/2017 8:07 PM, Michael Niedermayer wrote:
> > > On Sun, Aug 13, 2017 at 07:18:11PM -0300, James Almer wrote:
> > >> On 8/13/2017 7:15 PM, Michael Niedermayer wrote:
> > >>> Fixes: undefined behavior
> > >>>
> > >>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > >>> ---
> > >>>  libavcodec/rangecoder.c | 1 +
> > >>>  libavcodec/rangecoder.h | 8 ++++++--
> > >>>  2 files changed, 7 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/libavcodec/rangecoder.c b/libavcodec/rangecoder.c
> > >>> index 0bb79c880e..0d53bef076 100644
> > >>> --- a/libavcodec/rangecoder.c
> > >>> +++ b/libavcodec/rangecoder.c
> > >>> @@ -58,6 +58,7 @@ av_cold void ff_init_range_decoder(RangeCoder *c, const uint8_t *buf,
> > >>>  
> > >>>      c->low         = AV_RB16(c->bytestream);
> > >>>      c->bytestream += 2;
> > >>> +    c->overread    = 0;
> > >>>      if (c->low >= 0xFF00) {
> > >>>          c->low = 0xFF00;
> > >>>          c->bytestream_end = c->bytestream;
> > >>> diff --git a/libavcodec/rangecoder.h b/libavcodec/rangecoder.h
> > >>> index c3e81d0dcb..44af88b8f5 100644
> > >>> --- a/libavcodec/rangecoder.h
> > >>> +++ b/libavcodec/rangecoder.h
> > >>> @@ -42,6 +42,8 @@ typedef struct RangeCoder {
> > >>>      uint8_t *bytestream_start;
> > >>>      uint8_t *bytestream;
> > >>>      uint8_t *bytestream_end;
> > >>> +    int overread;
> > >>> +#define MAX_OVERREAD 2
> > >>>  } RangeCoder;
> > >>>  
> > >>>  void ff_init_range_encoder(RangeCoder *c, uint8_t *buf, int buf_size);
> > >>> @@ -106,9 +108,11 @@ static inline void refill(RangeCoder *c)
> > >>>      if (c->range < 0x100) {
> > >>>          c->range <<= 8;
> > >>>          c->low   <<= 8;
> > >>> -        if (c->bytestream < c->bytestream_end)
> > >>> +        if (c->bytestream < c->bytestream_end) {
> > >>>              c->low += c->bytestream[0];
> > >>> -        c->bytestream++;
> > >>> +            c->bytestream++;
> > >>> +        } else
> > >>> +            c->overread ++;
> > >>>      }
> > >>>  }
> > >>
> > >> Wouldn't it be better to port this to the bytestream2 reading api?
> > > 
> > > this is speed relevant code, i am not sure bytestream2 wouldnt add
> > > overhead. In fact i wasnt entirely sure keeping track of the overread
> > > bytes is a great idea. But i didnt see an easy way to avoid that.
> > 
> > Probably no overhead, since renorm_encoder() can use the unchecked
> > reader much like it's doing it now.
> > 
> > In any case, the buffer may be both read and written to, which means
> > you'd have to split things in two to use both the put and get
> > bytestream2 APIs, so it may not be worth doing (and probably the reason
> > why it's not already been done in the first place).
> 
> yes, so unless there are objections i will apply this in a day or 2

applied

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.
-------------- 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/20170818/57efcbe6/attachment.sig>