[FFmpeg-devel] [HACK] fix CAVS decoder crashes
Reimar Döffinger
Reimar.Doeffinger
Sun Dec 19 12:56:03 CET 2010
On Sat, Dec 18, 2010 at 02:44:40AM +0100, Michael Niedermayer wrote:
> On Sun, Dec 12, 2010 at 05:04:58PM +0100, Reimar D?ffinger wrote:
> > Hello,
> > I have the suspicion this decoder needs heavy fuzzing testing.
> > Anyway, trying to play http://samples.mplayerhq.hu/AVS/AVSFileFormat/AVSFileFormat.es
> > results in crashes which below hack "fixes".
> > Index: libavcodec/cavs.h
> > ===================================================================
> > --- libavcodec/cavs.h (revision 25928)
> > +++ libavcodec/cavs.h (working copy)
> > @@ -242,6 +242,7 @@
> > extern const cavs_vector ff_cavs_dir_mv;
> >
> > static inline void modify_pred(const int_fast8_t *mod_table, int *mode) {
> > + if (*mode < 0) *mode = 0;
> > *mode = mod_table[*mode];
> > if(*mode < 0) {
> > av_log(NULL, AV_LOG_ERROR, "Illegal intra prediction mode\n");
> > Index: libavcodec/cavsdec.c
> > ===================================================================
> > --- libavcodec/cavsdec.c (revision 25928)
> > +++ libavcodec/cavsdec.c (working copy)
> > @@ -122,7 +122,7 @@
> >
> > for(i=0;i<65;i++) {
> > level_code = get_ue_code(gb,r->golomb_order);
> > - if(level_code >= ESCAPE_CODE) {
> > + if(level_code >= ESCAPE_CODE || level_code < 0) {
> > run = ((level_code - ESCAPE_CODE) >> 1) + 1;
> > esc_code = get_ue_code(gb,esc_golomb_order);
> > level = esc_code + (run > r->max_run ? 1 : r->level_add[run]);
>
> > @@ -234,7 +234,7 @@
> > for(block=0;block<4;block++) {
> > d = h->cy + h->luma_scan[block];
> > ff_cavs_load_intra_pred_luma(h, top, &left, block);
> > - h->intra_pred_l[h->pred_mode_Y[ff_cavs_scan3x3[block]]]
> > + h->intra_pred_l[FFMAX(h->pred_mode_Y[ff_cavs_scan3x3[block]], 0)]
>
> as stefan has no time, some comments that might help debuging
> The intra pred stuff (likely) has to be >= 0 for intra blocks
> It (likely) is allowed to be negative for non intra blocks
> i suspect the bug is where this becomes inconsistent (aka intra with negative)
> a few asserts() might help finding this
They actually seem to come from uninitialized memory, however valgrind had such
an insane amount of issue with this code that I first had to fix loads of other stuff
(most of the input validity checks were just not working, and I doubt I caught all).
Note that using mallocz is definitely wrong, some of those need to be initialized
to e.g. -1.
Index: ffmpeg/libavcodec/cavs.c
===================================================================
--- ffmpeg/libavcodec/cavs.c (revision 25928)
+++ ffmpeg/libavcodec/cavs.c (working copy)
@@ -577,6 +577,8 @@
int ff_cavs_next_mb(AVSContext *h) {
int i;
+ if (get_bits_left(&h->s.gb) < 0)
+ return 0;
h->flags |= A_AVAIL;
h->cy += 16;
h->cu += 8;
@@ -653,17 +655,17 @@
*/
void ff_cavs_init_top_lines(AVSContext *h) {
/* alloc top line of predictors */
- h->top_qp = av_malloc( h->mb_width);
- h->top_mv[0] = av_malloc((h->mb_width*2+1)*sizeof(cavs_vector));
- h->top_mv[1] = av_malloc((h->mb_width*2+1)*sizeof(cavs_vector));
- h->top_pred_Y = av_malloc( h->mb_width*2*sizeof(*h->top_pred_Y));
- h->top_border_y = av_malloc((h->mb_width+1)*16);
- h->top_border_u = av_malloc((h->mb_width)*10);
- h->top_border_v = av_malloc((h->mb_width)*10);
+ h->top_qp = av_mallocz( h->mb_width);
+ h->top_mv[0] = av_mallocz((h->mb_width*2+1)*sizeof(cavs_vector));
+ h->top_mv[1] = av_mallocz((h->mb_width*2+1)*sizeof(cavs_vector));
+ h->top_pred_Y = av_mallocz( h->mb_width*2*sizeof(*h->top_pred_Y));
+ h->top_border_y = av_mallocz((h->mb_width+1)*16);
+ h->top_border_u = av_mallocz((h->mb_width)*10);
+ h->top_border_v = av_mallocz((h->mb_width)*10);
/* alloc space for co-located MVs and types */
- h->col_mv = av_malloc( h->mb_width*h->mb_height*4*sizeof(cavs_vector));
- h->col_type_base = av_malloc(h->mb_width*h->mb_height);
+ h->col_mv = av_mallocz( h->mb_width*h->mb_height*4*sizeof(cavs_vector));
+ h->col_type_base = av_mallocz(h->mb_width*h->mb_height);
h->block = av_mallocz(64*sizeof(DCTELEM));
}
Index: ffmpeg/libavcodec/cavsdec.c
===================================================================
--- ffmpeg/libavcodec/cavsdec.c (revision 25928)
+++ ffmpeg/libavcodec/cavsdec.c (working copy)
@@ -115,7 +115,8 @@
static int decode_residual_block(AVSContext *h, GetBitContext *gb,
const struct dec_2dvlc *r, int esc_golomb_order,
int qp, uint8_t *dst, int stride) {
- int i, level_code, esc_code, level, run, mask;
+ int i, esc_code, level, mask;
+ unsigned level_code, run;
DCTELEM level_buf[65];
uint8_t run_buf[65];
DCTELEM *block = h->block;
@@ -162,7 +163,7 @@
int block;
/* get coded block pattern */
- int cbp= get_ue_golomb(&h->s.gb);
+ unsigned cbp= get_ue_golomb(&h->s.gb);
if(cbp > 63){
av_log(h->s.avctx, AV_LOG_ERROR, "illegal inter cbp\n");
return -1;
@@ -187,9 +188,10 @@
*
****************************************************************************/
-static int decode_mb_i(AVSContext *h, int cbp_code) {
+static int decode_mb_i(AVSContext *h, unsigned cbp_code) {
GetBitContext *gb = &h->s.gb;
- int block, pred_mode_uv;
+ int block;
+ unsigned pred_mode_uv;
uint8_t top[18];
uint8_t *left = NULL;
uint8_t *d;
@@ -557,11 +559,11 @@
skip_count = -1;
if(h->skip_mode_flag && (skip_count < 0))
skip_count = get_ue_golomb(&s->gb);
- if(h->skip_mode_flag && skip_count--) {
+ if(h->skip_mode_flag && skip_count-- > 0) {
decode_mb_p(h,P_SKIP);
} else {
mb_type = get_ue_golomb(&s->gb) + P_SKIP + h->skip_mode_flag;
- if(mb_type > P_8X8)
+ if((unsigned)mb_type > P_8X8)
decode_mb_i(h, mb_type - P_8X8 - 1);
else
decode_mb_p(h,mb_type);
@@ -573,11 +575,11 @@
skip_count = -1;
if(h->skip_mode_flag && (skip_count < 0))
skip_count = get_ue_golomb(&s->gb);
- if(h->skip_mode_flag && skip_count--) {
+ if(h->skip_mode_flag && skip_count-- > 0) {
decode_mb_b(h,B_SKIP);
} else {
mb_type = get_ue_golomb(&s->gb) + B_SKIP + h->skip_mode_flag;
- if(mb_type > B_8X8)
+ if((unsigned)mb_type > B_8X8)
decode_mb_i(h, mb_type - B_8X8 - 1);
else
decode_mb_b(h,mb_type);
More information about the ffmpeg-devel
mailing list