Fix potential use-after-free bug in VideoImageGenerator::onGetYUV8Planes.

media/filters/skcanvas_video_renderer.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/filters/skcanvas_video_renderer.h"
 
 #include "base/logging.h"
 #include "media/base/video_frame.h"
 #include "media/base/yuv_convert.h"
 #include "third_party/libyuv/include/libyuv.h"
@@ skipping 190 matching lines @@
     }
     default:
       NOTREACHED();
       break;
   }
 }
 
 // Generates an RGB image from a VideoFrame.
 class VideoImageGenerator : public SkImageGenerator {
  public:
-  VideoImageGenerator(const scoped_refptr<VideoFrame>& frame) : frame_(frame) {}
+  VideoImageGenerator(const scoped_refptr<VideoFrame>& frame) : frame_(frame) {
+    DCHECK(frame_.get());
+  }
   virtual ~VideoImageGenerator() {}
 
   void set_frame(const scoped_refptr<VideoFrame>& frame) { frame_ = frame; }
 
  protected:
   virtual bool onGetInfo(SkImageInfo* info) override {
     info->fWidth = frame_->visible_rect().width();
     info->fHeight = frame_->visible_rect().height();
     info->fColorType = kN32_SkColorType;
     info->fAlphaType = kPremul_SkAlphaType;
     return true;
   }
 
   virtual bool onGetPixels(const SkImageInfo& info,
                            void* pixels,
                            size_t row_bytes,
                            SkPMColor ctable[],
                            int* ctable_count) override {
     if (!frame_.get())
       return false;
     if (!pixels)
-      return true;
+      return false;
     // If skia couldn't do the YUV conversion, we will.
     ConvertVideoFrameToRGBPixels(frame_, pixels, row_bytes);
-    frame_ = NULL;
     return true;
   }
 
   virtual bool onGetYUV8Planes(SkISize sizes[3],
                                void* planes[3],
                                size_t row_bytes[3],
                                SkYUVColorSpace* color_space) override {
     if (!frame_.get() || !IsYUV(frame_->format()))
       return false;
 
@@ skipping 24 matching lines @@
                    frame_->visible_rect().x();
         } else {
           offset = (frame_->stride(media::VideoFrame::kUPlane) *
                     (frame_->visible_rect().y() >> y_shift)) +
                    (frame_->visible_rect().x() >> 1);
         }
         row_bytes[plane] = static_cast<size_t>(frame_->stride(plane));
         planes[plane] = frame_->data(plane) + offset;
       }
     }
-    if (planes && row_bytes)
-      frame_ = NULL;
     return true;
   }
 
  private:
   scoped_refptr<VideoFrame> frame_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(VideoImageGenerator);
 };
 
 SkCanvasVideoRenderer::SkCanvasVideoRenderer()
     : generator_(NULL), last_frame_timestamp_(media::kNoTimestamp()) {
   last_frame_.setIsVolatile(true);
 }
 
 SkCanvasVideoRenderer::~SkCanvasVideoRenderer() {}
 
 void SkCanvasVideoRenderer::Paint(const scoped_refptr<VideoFrame>& video_frame,
@@ skipping 27 matching lines @@
 
     // Note: This takes ownership of |generator_|.
     if (!SkInstallDiscardablePixelRef(generator_, &last_frame_)) {
       NOTREACHED();
     }
     DCHECK(video_frame->visible_rect().width() == last_frame_.width() &&
            video_frame->visible_rect().height() == last_frame_.height());
 
     last_frame_timestamp_ = video_frame->timestamp();
   } else if (generator_) {
     generator_->set_frame(video_frame);

[dshwang, 2014/10/10 16:50:20]

This line is needed because |generator_| generates pixel or texture every time
when |last_frame_| is used. As the name "discardable pixel ref", |last_frame_|
can cache or not the converted pixel or texture. If |last_frame_| discard the
pixel, |generator_| needs the video frame again.

   }
 
   paint.setXfermodeMode(mode);
   paint.setFilterLevel(SkPaint::kLow_FilterLevel);
 
   bool need_transform =
       video_rotation != VIDEO_ROTATION_0 ||
       dest_rect.size() != video_frame->visible_rect().size() ||
       !dest_rect.origin().IsOrigin();
   if (need_transform) {
@@ skipping 26 matching lines @@
     canvas->scale(
         SkFloatToScalar(rotated_dest_size.width() / last_frame_.width()),
         SkFloatToScalar(rotated_dest_size.height() / last_frame_.height()));
     canvas->translate(-SkFloatToScalar(last_frame_.width() * 0.5f),
                       -SkFloatToScalar(last_frame_.height() * 0.5f));
   }
   canvas->drawBitmap(last_frame_, 0, 0, &paint);
   if (need_transform)
     canvas->restore();
   canvas->flush();
+  // SkCanvas::flush() causes the generator to generate SkImage, so delete
+  // |video_frame| not to be outlived.
+  if (generator_)
+    generator_->set_frame(NULL);
 }
 
 void SkCanvasVideoRenderer::Copy(const scoped_refptr<VideoFrame>& video_frame,
                                  SkCanvas* canvas) {
   Paint(video_frame,
         canvas,
         video_frame->visible_rect(),
         0xff,
         SkXfermode::kSrc_Mode,
         media::VIDEO_ROTATION_0);
 }
 
 }  // namespace media