* [PATCH v3 0/2] media: videobuf2: make sure bytesused is smaller than the buffer size
@ 2021-12-01 22:56 Dafna Hirschfeld
2021-12-01 22:56 ` [PATCH v3 1/2] media: replace setting of bytesused with vb2_set_plane_payload Dafna Hirschfeld
2021-12-01 22:56 ` [PATCH v3 2/2] media: videobuf2: add WARN_ON_ONCE if bytesused is bigger than buffer length Dafna Hirschfeld
0 siblings, 2 replies; 3+ messages in thread
From: Dafna Hirschfeld @ 2021-12-01 22:56 UTC (permalink / raw)
To: linux-media
Cc: Dafna Hirschfeld, laurent.pinchart, hverkuil, kernel, dafna3,
sakari.ailus, mchehab
Add a WARN_ON_ONCE in vb2_set_plane_payload if bytesused is bigger than length
and clamp the bytesused to length.
Also change places where bytesused is set directly with that function.
This help find/eliminate possible buffer overflow.
changes since v2:
* Fix compilations issues on drivers/staging/media/meson/vdec/vdec_helpers.c
* clamp the bytesused to the buffer length if it is bigger
* update subject of second commit WARN_ON -> WARN_ON_ONCE
changes since v1:
* replace WARN_ON with WARN_ON_ONCE
* add inline doc
Dafna Hirschfeld (2):
media: replace setting of bytesused with vb2_set_plane_payload
media: videobuf2: add WARN_ON_ONCE if bytesused is bigger than buffer
length
drivers/media/platform/allegro-dvt/allegro-core.c | 2 +-
drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c | 10 +++++-----
drivers/media/test-drivers/vicodec/vicodec-core.c | 2 +-
drivers/media/usb/go7007/go7007-driver.c | 2 +-
drivers/staging/media/meson/vdec/vdec_helpers.c | 10 +++++-----
include/media/videobuf2-core.h | 9 ++++++++-
6 files changed, 21 insertions(+), 14 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v3 1/2] media: replace setting of bytesused with vb2_set_plane_payload
2021-12-01 22:56 [PATCH v3 0/2] media: videobuf2: make sure bytesused is smaller than the buffer size Dafna Hirschfeld
@ 2021-12-01 22:56 ` Dafna Hirschfeld
2021-12-01 22:56 ` [PATCH v3 2/2] media: videobuf2: add WARN_ON_ONCE if bytesused is bigger than buffer length Dafna Hirschfeld
1 sibling, 0 replies; 3+ messages in thread
From: Dafna Hirschfeld @ 2021-12-01 22:56 UTC (permalink / raw)
To: linux-media
Cc: Dafna Hirschfeld, laurent.pinchart, hverkuil, kernel, dafna3,
sakari.ailus, mchehab
In many places the bytesused field of struct vb2_buffer is set
directly. Replace that with the function call
vb2_set_plane_payload
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/allegro-dvt/allegro-core.c | 2 +-
drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c | 10 +++++-----
drivers/media/test-drivers/vicodec/vicodec-core.c | 2 +-
drivers/media/usb/go7007/go7007-driver.c | 2 +-
drivers/staging/media/meson/vdec/vdec_helpers.c | 10 +++++-----
5 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/allegro-core.c
index c8156da33043..4a3d06c70e34 100644
--- a/drivers/media/platform/allegro-dvt/allegro-core.c
+++ b/drivers/media/platform/allegro-dvt/allegro-core.c
@@ -2815,7 +2815,7 @@ static void allegro_buf_queue(struct vb2_buffer *vb)
unsigned int i;
for (i = 0; i < vb->num_planes; i++)
- vb->planes[i].bytesused = 0;
+ vb2_set_plane_payload(vb, i, 0);
vbuf->field = V4L2_FIELD_NONE;
vbuf->sequence = channel->csequence++;
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
index 7232dc053c64..d7b8223e2362 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
@@ -966,7 +966,7 @@ static void vb2ops_venc_stop_streaming(struct vb2_queue *q)
if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
while ((dst_buf = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx))) {
- dst_buf->vb2_buf.planes[0].bytesused = 0;
+ vb2_set_plane_payload(&dst_buf->vb2_buf, 0, 0);
v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_ERROR);
}
/* STREAMOFF on the CAPTURE queue completes any ongoing flush */
@@ -1075,7 +1075,7 @@ static int mtk_venc_encode_header(void *priv)
NULL, &bs_buf, &enc_result);
if (ret) {
- dst_buf->vb2_buf.planes[0].bytesused = 0;
+ vb2_set_plane_payload(&dst_buf->vb2_buf, 0, 0);
ctx->state = MTK_STATE_ABORT;
v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_ERROR);
mtk_v4l2_err("venc_if_encode failed=%d", ret);
@@ -1090,7 +1090,7 @@ static int mtk_venc_encode_header(void *priv)
}
ctx->state = MTK_STATE_HEADER;
- dst_buf->vb2_buf.planes[0].bytesused = enc_result.bs_size;
+ vb2_set_plane_payload(&dst_buf->vb2_buf, 0, enc_result.bs_size);
v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_DONE);
return 0;
@@ -1239,12 +1239,12 @@ static void mtk_venc_worker(struct work_struct *work)
if (ret) {
v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_ERROR);
- dst_buf->vb2_buf.planes[0].bytesused = 0;
+ vb2_set_plane_payload(&dst_buf->vb2_buf, 0, 0);
v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_ERROR);
mtk_v4l2_err("venc_if_encode failed=%d", ret);
} else {
v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_DONE);
- dst_buf->vb2_buf.planes[0].bytesused = enc_result.bs_size;
+ vb2_set_plane_payload(&dst_buf->vb2_buf, 0, enc_result.bs_size);
v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_DONE);
mtk_v4l2_debug(2, "venc_if_encode bs size=%d",
enc_result.bs_size);
diff --git a/drivers/media/test-drivers/vicodec/vicodec-core.c b/drivers/media/test-drivers/vicodec/vicodec-core.c
index 33f1c893c1b6..be43f7d32df9 100644
--- a/drivers/media/test-drivers/vicodec/vicodec-core.c
+++ b/drivers/media/test-drivers/vicodec/vicodec-core.c
@@ -1443,7 +1443,7 @@ static void vicodec_buf_queue(struct vb2_buffer *vb)
unsigned int i;
for (i = 0; i < vb->num_planes; i++)
- vb->planes[i].bytesused = 0;
+ vb2_set_plane_payload(vb, i, 0);
vbuf->field = V4L2_FIELD_NONE;
vbuf->sequence =
diff --git a/drivers/media/usb/go7007/go7007-driver.c b/drivers/media/usb/go7007/go7007-driver.c
index 6650eab913d8..0c24e2984304 100644
--- a/drivers/media/usb/go7007/go7007-driver.c
+++ b/drivers/media/usb/go7007/go7007-driver.c
@@ -516,7 +516,7 @@ void go7007_parse_video_stream(struct go7007 *go, u8 *buf, int length)
if (vb && vb->vb.vb2_buf.planes[0].bytesused >=
GO7007_BUF_SIZE - 3) {
v4l2_info(&go->v4l2_dev, "dropping oversized frame\n");
- vb->vb.vb2_buf.planes[0].bytesused = 0;
+ vb2_set_plane_payload(&vb->vb.vb2_buf, 0, 0);
vb->frame_offset = 0;
vb->modet_active = 0;
vb = go->active_buf = NULL;
diff --git a/drivers/staging/media/meson/vdec/vdec_helpers.c b/drivers/staging/media/meson/vdec/vdec_helpers.c
index b9125c295d1d..203d7afa085d 100644
--- a/drivers/staging/media/meson/vdec/vdec_helpers.c
+++ b/drivers/staging/media/meson/vdec/vdec_helpers.c
@@ -276,13 +276,13 @@ static void dst_buf_done(struct amvdec_session *sess,
switch (sess->pixfmt_cap) {
case V4L2_PIX_FMT_NV12M:
- vbuf->vb2_buf.planes[0].bytesused = output_size;
- vbuf->vb2_buf.planes[1].bytesused = output_size / 2;
+ vb2_set_plane_payload(&vbuf->vb2_buf, 0, output_size);
+ vb2_set_plane_payload(&vbuf->vb2_buf, 1, output_size / 2);
break;
case V4L2_PIX_FMT_YUV420M:
- vbuf->vb2_buf.planes[0].bytesused = output_size;
- vbuf->vb2_buf.planes[1].bytesused = output_size / 4;
- vbuf->vb2_buf.planes[2].bytesused = output_size / 4;
+ vb2_set_plane_payload(&vbuf->vb2_buf, 0, output_size);
+ vb2_set_plane_payload(&vbuf->vb2_buf, 1, output_size / 4);
+ vb2_set_plane_payload(&vbuf->vb2_buf, 2, output_size / 4);
break;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v3 2/2] media: videobuf2: add WARN_ON_ONCE if bytesused is bigger than buffer length
2021-12-01 22:56 [PATCH v3 0/2] media: videobuf2: make sure bytesused is smaller than the buffer size Dafna Hirschfeld
2021-12-01 22:56 ` [PATCH v3 1/2] media: replace setting of bytesused with vb2_set_plane_payload Dafna Hirschfeld
@ 2021-12-01 22:56 ` Dafna Hirschfeld
1 sibling, 0 replies; 3+ messages in thread
From: Dafna Hirschfeld @ 2021-12-01 22:56 UTC (permalink / raw)
To: linux-media
Cc: Dafna Hirschfeld, laurent.pinchart, hverkuil, kernel, dafna3,
sakari.ailus, mchehab
In function vb2_set_plane_payload, report if the
given bytesused is bigger than the buffer size,
and clamp it to the buffer size.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
include/media/videobuf2-core.h | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 2467284e5f26..5468b633b9d2 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -1155,8 +1155,15 @@ static inline void *vb2_get_drv_priv(struct vb2_queue *q)
static inline void vb2_set_plane_payload(struct vb2_buffer *vb,
unsigned int plane_no, unsigned long size)
{
- if (plane_no < vb->num_planes)
+ /*
+ * size must never be larger than the buffer length, so
+ * warn and clamp to the buffer length if that's the case.
+ */
+ if (plane_no < vb->num_planes) {
+ if (WARN_ON_ONCE(size > vb->planes[plane_no].length))
+ size = vb->planes[plane_no].length;
vb->planes[plane_no].bytesused = size;
+ }
}
/**
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-12-01 22:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-01 22:56 [PATCH v3 0/2] media: videobuf2: make sure bytesused is smaller than the buffer size Dafna Hirschfeld
2021-12-01 22:56 ` [PATCH v3 1/2] media: replace setting of bytesused with vb2_set_plane_payload Dafna Hirschfeld
2021-12-01 22:56 ` [PATCH v3 2/2] media: videobuf2: add WARN_ON_ONCE if bytesused is bigger than buffer length Dafna Hirschfeld
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).