From: Jerome Jiang <jianj@google.com>
Date: Tue, 7 Aug 2018 18:10:26 +0000 (-0700)
Subject: vp8: fix to address overflow in decoder.
X-Git-Tag: v1.8.0~187^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f3a027a46de103c97f9f413fea003dc3d97e2cfc;p=libvpx

vp8: fix to address overflow in decoder.

Can't call internal error from the decoder thread.

Add vpx_internal_error_info to MACROBLOCKD. When corrupted frame
detected, the decoder thread returns to its own context and signal
completion of decoding for current frame.

The main decoding thread will detect error too and return error code to
decoding API call.

Each thread will signal end of decoding of the frame. Main thread waits
for the signal of all other threads to start decoding next frame.

BUG=875626,webm:1496
Change-Id: Icd05fbc558893a4e7d8532c1e7177e7550283a64
---

diff --git a/vp8/common/blockd.h b/vp8/common/blockd.h
index 22af8980b..cb8de0caf 100644
--- a/vp8/common/blockd.h
+++ b/vp8/common/blockd.h
@@ -13,6 +13,7 @@
 
 void vpx_log(const char *format, ...);
 
+#include "vpx/internal/vpx_codec_internal.h"
 #include "vpx_config.h"
 #include "vpx_scale/yv12config.h"
 #include "mv.h"
@@ -288,6 +289,8 @@ typedef struct macroblockd {
 
   int corrupted;
 
+  struct vpx_internal_error_info error_info;
+
 #if ARCH_X86 || ARCH_X86_64
   /* This is an intermediate buffer currently used in sub-pixel motion search
    * to keep a copy of the reference area. This buffer can be used for other
diff --git a/vp8/decoder/decodeframe.c b/vp8/decoder/decodeframe.c
index 82b72d21e..f4842ef03 100644
--- a/vp8/decoder/decodeframe.c
+++ b/vp8/decoder/decodeframe.c
@@ -1220,7 +1220,8 @@ int vp8_decode_frame(VP8D_COMP *pbi) {
   if (vpx_atomic_load_acquire(&pbi->b_multithreaded_rd) &&
       pc->multi_token_partition != ONE_PARTITION) {
     unsigned int thread;
-    vp8mt_decode_mb_rows(pbi, xd);
+    if (vp8mt_decode_mb_rows(pbi, xd))
+      vpx_internal_error(&pbi->common.error, VPX_CODEC_CORRUPT_FRAME, NULL);
     vp8_yv12_extend_frame_borders(yv12_fb_new);
     for (thread = 0; thread < pbi->decoding_thread_count; ++thread) {
       corrupt_tokens |= pbi->mb_row_di[thread].mbd.corrupted;
diff --git a/vp8/decoder/decoderthreading.h b/vp8/decoder/decoderthreading.h
index 140295625..3d49bc831 100644
--- a/vp8/decoder/decoderthreading.h
+++ b/vp8/decoder/decoderthreading.h
@@ -16,7 +16,7 @@ extern "C" {
 #endif
 
 #if CONFIG_MULTITHREAD
-void vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd);
+int vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd);
 void vp8_decoder_remove_threads(VP8D_COMP *pbi);
 void vp8_decoder_create_threads(VP8D_COMP *pbi);
 void vp8mt_alloc_temp_buffers(VP8D_COMP *pbi, int width, int prev_mb_rows);
diff --git a/vp8/decoder/onyxd_if.c b/vp8/decoder/onyxd_if.c
index f516eb0c7..2b8dd4ae2 100644
--- a/vp8/decoder/onyxd_if.c
+++ b/vp8/decoder/onyxd_if.c
@@ -331,6 +331,7 @@ int vp8dx_receive_compressed_data(VP8D_COMP *pbi, size_t size,
     if (cm->fb_idx_ref_cnt[cm->new_fb_idx] > 0) {
       cm->fb_idx_ref_cnt[cm->new_fb_idx]--;
     }
+    pbi->common.error.setjmp = 0;
     goto decode_exit;
   }
 
@@ -344,6 +345,12 @@ int vp8dx_receive_compressed_data(VP8D_COMP *pbi, size_t size,
     }
 
     pbi->common.error.error_code = VPX_CODEC_ERROR;
+    // Propagate the error info.
+    if (pbi->mb.error_info.error_code != 0) {
+      pbi->common.error.error_code = pbi->mb.error_info.error_code;
+      memcpy(pbi->common.error.detail, pbi->mb.error_info.detail,
+             sizeof(pbi->mb.error_info.detail));
+    }
     goto decode_exit;
   }
 
@@ -382,7 +389,6 @@ int vp8dx_receive_compressed_data(VP8D_COMP *pbi, size_t size,
   pbi->last_time_stamp = time_stamp;
 
 decode_exit:
-  pbi->common.error.setjmp = 0;
   vpx_clear_system_state();
   return retcode;
 }
diff --git a/vp8/decoder/threading.c b/vp8/decoder/threading.c
index aadc8dc71..8cc301d4f 100644
--- a/vp8/decoder/threading.c
+++ b/vp8/decoder/threading.c
@@ -400,6 +400,21 @@ static void mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd,
       xd->dst.u_buffer = dst_buffer[1] + recon_uvoffset;
       xd->dst.v_buffer = dst_buffer[2] + recon_uvoffset;
 
+      /* propagate errors from reference frames */
+      xd->corrupted |= ref_fb_corrupted[xd->mode_info_context->mbmi.ref_frame];
+
+      if (xd->corrupted) {
+        // Move current decoding marcoblock to the end of row for all rows
+        // assigned to this thread, such that other threads won't be waiting.
+        for (; mb_row < pc->mb_rows;
+             mb_row += (pbi->decoding_thread_count + 1)) {
+          current_mb_col = &pbi->mt_current_mb_col[mb_row];
+          vpx_atomic_store_release(current_mb_col, pc->mb_cols + nsync);
+        }
+        vpx_internal_error(&xd->error_info, VPX_CODEC_CORRUPT_FRAME,
+                           "Corrupted reference frame");
+      }
+
       xd->pre.y_buffer =
           ref_buffer[xd->mode_info_context->mbmi.ref_frame][0] + recon_yoffset;
       xd->pre.u_buffer =
@@ -407,9 +422,6 @@ static void mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd,
       xd->pre.v_buffer =
           ref_buffer[xd->mode_info_context->mbmi.ref_frame][2] + recon_uvoffset;
 
-      /* propagate errors from reference frames */
-      xd->corrupted |= ref_fb_corrupted[xd->mode_info_context->mbmi.ref_frame];
-
       mt_decode_macroblock(pbi, xd, 0);
 
       xd->left_available = 1;
@@ -557,8 +569,9 @@ static void mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd,
     xd->mode_info_context += xd->mode_info_stride * pbi->decoding_thread_count;
   }
 
-  /* signal end of frame decoding if this thread processed the last mb_row */
-  if (last_mb_row == (pc->mb_rows - 1)) sem_post(&pbi->h_event_end_decoding);
+  /* signal end of decoding of current thread for current frame */
+  if (last_mb_row + (int)pbi->decoding_thread_count + 1 >= pc->mb_rows)
+    sem_post(&pbi->h_event_end_decoding);
 }
 
 static THREAD_FUNCTION thread_decoding_proc(void *p_data) {
@@ -576,7 +589,13 @@ static THREAD_FUNCTION thread_decoding_proc(void *p_data) {
       } else {
         MACROBLOCKD *xd = &mbrd->mbd;
         xd->left_context = &mb_row_left_context;
-
+        if (setjmp(xd->error_info.jmp)) {
+          xd->error_info.setjmp = 0;
+          // Signal the end of decoding for current thread.
+          sem_post(&pbi->h_event_end_decoding);
+          continue;
+        }
+        xd->error_info.setjmp = 1;
         mt_decode_mb_rows(pbi, xd, ithread + 1);
       }
     }
@@ -809,7 +828,7 @@ void vp8_decoder_remove_threads(VP8D_COMP *pbi) {
   }
 }
 
-void vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd) {
+int vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd) {
   VP8_COMMON *pc = &pbi->common;
   unsigned int i;
   int j;
@@ -855,7 +874,22 @@ void vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd) {
     sem_post(&pbi->h_event_start_decoding[i]);
   }
 
+  if (setjmp(xd->error_info.jmp)) {
+    xd->error_info.setjmp = 0;
+    xd->corrupted = 1;
+    // Wait for other threads to finish. This prevents other threads decoding
+    // the current frame while the main thread starts decoding the next frame,
+    // which causes a data race.
+    for (i = 0; i < pbi->decoding_thread_count; ++i)
+      sem_wait(&pbi->h_event_end_decoding);
+    return -1;
+  }
+
+  xd->error_info.setjmp = 1;
   mt_decode_mb_rows(pbi, xd, 0);
 
-  sem_wait(&pbi->h_event_end_decoding); /* add back for each frame */
+  for (i = 0; i < pbi->decoding_thread_count + 1; ++i)
+    sem_wait(&pbi->h_event_end_decoding); /* add back for each frame */
+
+  return 0;
 }