drivers/media/pci/cx18/cx18-queue.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
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 leaves bytesused above
s->buf_size, and two paths then use it as a bound over a buffer of that
size.
cx18_buf_swap() walks the buffer in 32 bit words up to bytesused and
byteswaps them in place:
for (i = 0; i < buf->bytesused; i += 4)
swab32s((u32 *)(buf->buf + i));
That writes past the end of buf->buf. The call comes from
cx18_mdl_swap() for MPG streams when CX18_F_M_NEED_SWAP is set, which
cx18_queue_get_mdl() does for every type except TS.
cx18_copy_buf_to_user() computes the remaining length the same way:
size_t len = buf->bytesused - buf->readpos;
and copies it to userspace. Here len is capped at the count the reader
asked for, so the read is bounded by the read() size, but it still
starts from a bytesused that reaches past the 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>
Assisted-by: LLM
Signed-off-by: Guo Zihao <guozh23@xiaopeng.com>
---
v2: add the CX18 maintainer to the recipients, and describe the two
places that use bytesused as a bound rather than only the read path.
The previous version described the copy to userspace and called the
result an out of bounds read. cx18_buf_swap() uses the same value as
a loop bound and writes in place, so the description now covers that
path as well, and notes that the copy to userspace is capped by the
read size whereas the byteswap loop is not.
Add the Assisted-by tag.
The firmware reports the length in the ACK. A device that reports a
data_used larger than the buffer it filled leaves bytesused above
s->buf_size, and the single buffer path is the one that does not bound
it. The clamp here is the same one the multi buffer path applies.
cx18_buf_swap() itself is left as it is: the bound it needs is the buffer
size, which the clamp now keeps bytesused below.
---
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
© 2016 - 2026 Red Hat, Inc.