[PATCH] media: cx18: clamp bytesused in the single buffer MDL path

Guo Zihao posted 1 patch 6 days, 17 hours ago
There is a newer version of this series
drivers/media/pci/cx18/cx18-queue.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[PATCH] media: cx18: clamp bytesused in the single buffer MDL path
Posted by Guo Zihao 6 days, 17 hours ago
cx18_mdl_update_bufs_for_cpu() copies mdl->bytesused straight into the
buffer when the MDL holds a single buffer:

        if (list_is_singular(&mdl->buf_list)) {
                buf = list_first_entry(&mdl->buf_list, struct cx18_buffer,
                                       list);
                buf->bytesused = mdl->bytesused;

The value originates from the firmware. cx18_queue_get_mdl() receives it
as its bytesused argument and stores it in ret->bytesused; the mailbox
handler calls it as cx18_queue_get_mdl(s, id, mdl_ack->data_used), so
data_used from the firmware ACK is what ends up in the buffer. Firmware
that reports more than the buffer holds therefore reaches
cx18_copy_buf_to_user(), where

        size_t len = buf->bytesused - buf->readpos;

turns into a read past the end of the DMA buffer.

The multi buffer path already guards this: _cx18_mdl_update_bufs_for_cpu()
clamps each buffer to s->buf_size before assigning. Apply the same clamp
in the single buffer case.

The single buffer path is the common one here, not a corner case:
cx18_stream_init() sets bufs_per_mdl to 1, so list_is_singular() is
normally true.

No Fixes tag. Both paths were introduced together with the driver
(1c1e45d17b66, "V4L/DVB (7786): cx18: new driver for the Conexant
CX23418 MPEG"), and the clamp in the multi buffer path was added later,
leaving this one behind.

Reviewed-by: Liu Chao <liuc63@xiaopeng.com>
Signed-off-by: Guo Zihao <guozh23@xiaopeng.com>
---
Reaching this needs firmware that reports a data_used larger than the
buffer it filled. The two paths then assign the same value and only one
of them bounds it.

The impact is a read past the DMA buffer rather than a write, and the
surplus data is handed to the reader of the stream. The clamp in the
multi buffer path keeps that case inside s->buf_size; the single buffer
path has no such bound.

 drivers/media/pci/cx18/cx18-queue.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/cx18/cx18-queue.c b/drivers/media/pci/cx18/cx18-queue.c
index 04d6828f0..a973367d4 100644
--- a/drivers/media/pci/cx18/cx18-queue.c
+++ b/drivers/media/pci/cx18/cx18-queue.c
@@ -114,7 +114,10 @@ static inline void cx18_mdl_update_bufs_for_cpu(struct cx18_stream *s,
 	if (list_is_singular(&mdl->buf_list)) {
 		buf = list_first_entry(&mdl->buf_list, struct cx18_buffer,
 				       list);
-		buf->bytesused = mdl->bytesused;
+		if (mdl->bytesused > s->buf_size)
+			buf->bytesused = s->buf_size;
+		else
+			buf->bytesused = mdl->bytesused;
 		buf->readpos = 0;
 		cx18_buf_sync_for_cpu(s, buf);
 	} else {
-- 
2.50.1