[bug report] [media] vcodec: mediatek: Add Mediatek V4L2 Video Decoder Driver

[Dan Carpenter]

Hello Tiffany Lin,

The patch 590577a4e525: "[media] vcodec: mediatek: Add Mediatek V4L2
Video Decoder Driver" from Sep 2, 2016, leads to the following static
checker warning:

	drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c:536 vidioc_vdec_qbuf()
	error: buffer overflow 'vq->bufs' 32 <= u32max

drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
   520  static int vidioc_vdec_qbuf(struct file *file, void *priv,
   521                              struct v4l2_buffer *buf)
   522  {
   523          struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
   524          struct vb2_queue *vq;
   525          struct vb2_buffer *vb;
   526          struct mtk_video_dec_buf *mtkbuf;
   527          struct vb2_v4l2_buffer  *vb2_v4l2;
   528  
   529          if (ctx->state == MTK_STATE_ABORT) {
   530                  mtk_v4l2_err("[%d] Call on QBUF after unrecoverable error",
   531                                  ctx->id);
   532                  return -EIO;
   533          }
   534  
   535          vq = v4l2_m2m_get_vq(ctx->m2m_ctx, buf->type);
   536          vb = vq->bufs[buf->index];

Smatch thinks that "buf->index" comes straight from the user without
being checked and that this is a buffer overflow.  It seems simple
enough to analyse the call tree.

__video_do_ioctl()
->  v4l_qbuf()
  -> vidioc_vdec_qbuf()

It seems like Smatch is correct.  I looked at a different implementation
of this and that one wasn't checked either so maybe there is something
I am not seeing.

This has obvious security implications.  Can someone take a look at
this?

   537          vb2_v4l2 = container_of(vb, struct vb2_v4l2_buffer, vb2_buf);
   538          mtkbuf = container_of(vb2_v4l2, struct mtk_video_dec_buf, vb);
   539  
   540          if ((buf->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) &&
   541              (buf->m.planes[0].bytesused == 0)) {
   542                  mtkbuf->lastframe = true;
   543                  mtk_v4l2_debug(1, "[%d] (%d) id=%d lastframe=%d (%d,%d, %d) vb=%p",
   544                           ctx->id, buf->type, buf->index,
   545                           mtkbuf->lastframe, buf->bytesused,
   546                           buf->m.planes[0].bytesused, buf->length,
   547                           vb);
   548          }
   549  
   550          return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
   551  }

regards,
dan carpenter