Re: [bug report] [media] s5p-mfc: use MFC_BUF_FLAG_EOS to identify last buffers in decoder capture queue

[Andrzej Hajda <a.hajda@samsung.com>]

Hi Dan,

On 04.04.2019 17:42, Dan Carpenter wrote:
> Hello Andrzej Hajda,
>
> The patch 4d0b0ed63660: "[media] s5p-mfc: use MFC_BUF_FLAG_EOS to
> identify last buffers in decoder capture queue" from Oct 7, 2015,
> leads to the following static checker warning:
>
> 	drivers/media/platform/s5p-mfc/s5p_mfc_dec.c:664 vidioc_dqbuf()
> 	warn: uncapped user index 'ctx->dst_bufs[buf->index]'


Almost identical e-mail you have sent about year ago, and me and Hans
have explained you that it is false positive.

Has something changed?


Regards

Andrzej


>
> drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
>     642 static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
>     643 {
>     644 	const struct v4l2_event ev = {
>     645 		.type = V4L2_EVENT_EOS
>     646 	};
>     647 	struct s5p_mfc_ctx *ctx = fh_to_ctx(priv);
>     648 	int ret;
>     649 
>     650 	if (ctx->state == MFCINST_ERROR) {
>     651 		mfc_err_limited("Call on DQBUF after unrecoverable error\n");
>     652 		return -EIO;
>     653 	}
>     654 
>     655 	switch (buf->type) {
>     656 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>     657 		return vb2_dqbuf(&ctx->vq_src, buf, file->f_flags & O_NONBLOCK);
>     658 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>     659 		ret = vb2_dqbuf(&ctx->vq_dst, buf, file->f_flags & O_NONBLOCK);
>     660 		if (ret)
>     661 			return ret;
>     662 
>     663 		if (ctx->state == MFCINST_FINISHED &&
> --> 664 		    (ctx->dst_bufs[buf->index].flags & MFC_BUF_FLAG_EOS))
>                                            ^^^^^^^^^^
> Smatch is saying that this isn't capped.  The truth is that v4l2 code is
> a bit complicated for Smatch, but in this case I can't see where
> "buf->index" does get capped.  I would have expected it to be capped in
> check_array_args() where we check "buf->length" but it's not.
>
> I've been going through these warnings really carefully in the past
> couple weeks trying to fix false positives so let me know what I'm
> missing and I will update the check.  Even if I have to manually muck
> in the DB.
>
>     665 			v4l2_event_queue_fh(&ctx->fh, &ev);
>     666 		return 0;
>     667 	default:
>     668 		return -EINVAL;
>     669 	}
>     670 }
>
> regards,
> dan carpenter
>
>