* [PATCH] media: dvb-core: fix OOB read in dtv_property_process_get()
@ 2026-10-09 14:23 Mirza Ishan Beg
0 siblings, 0 replies; only message in thread
From: Mirza Ishan Beg @ 2026-10-09 14:23 UTC (permalink / raw)
To: linux-media
Cc: linux-kernel, Mauro Carvalho Chehab, Hans Verkuil, Kees Cook,
Slawomir Stepien, Mirza Ishan Beg, syzbot+6102f1c3a0ea8073fd64
dtv_property_process_get() switches on 'tvp->cmd' and, in each case,
writes a different member of the union 'u' in struct dtv_property.
'u.data' carries the scalar commands, while u.st carries the
'DTV_STAT_*' commands, and u.buffer only DTV_ENUM_DELSYS.
After the switch, a single shared dev_dbg() reads 'u.buffer.len'
and 'u.buffer.data' via %*ph, regardless of which member the case
wrote.
struct 'dtv_property' is 76 bytes and union 'u' begins at offset 16:
u.data : 4 bytes at 16
u.st : 37 bytes at 16
u.buffer : 56 bytes at 16 ('data[32]' at 16-47, 'len' at 48-51)
For commands that only write 'u.data', 'u.buffer.len' is never set by
the kernel and keeps whatever userspace supplied to FE_GET_PROPERTY,
because dvb_get_property() copies the array in with
memdup_array_user() and does not clear the union. A large value (the
reproducer uses 1163001896) makes vsprintf() emit "field width ...
out of range" and hex_string() clamp the length to 64 bytes. The
read starts at 'u.buffer.data', offset 16, so 64 bytes reach offset 80
and KASAN reports a slab-out-of-bounds read past the 76-byte object.
The DTV_STAT_* cases hit the same shared print. 'u.buffer.len' overlaps
'u.st.stat[3].uvalue', so the debug output is meaningless even when the
read stays in bounds. The same applies to the clamp added by
60f0618d157b, which writes to 'u.buffer.len' and therefore into the
just-written stats payload.
Fix by storing the point of writing which union member each case
populated, and by branching the final dev_dbg() on that stored point.
Add a local enum 'dtv_get_shape' with values 'SHAPE_DATA', 'SHAPE_ST'
and 'SHAPE_BUFFER', initialised to 'SHAPE_DATA' and set in the
'DTV_ENUM_DELSYS' and 'DTV_STAT_*' cases. The final print then reads only
the member the case actually wrote, and every branch stays inside the
76-byte object. The clamp and the now-unused 'len' variable are removed.
Fixes: 60f0618d157b ("media: dvb-core: frontend: make GET/SET safer")
Suggested-by: Slawomir Stepien <sst@poczta.fm>
Reported-by: syzbot+6102f1c3a0ea8073fd64@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6102f1c3a0ea8073fd64
Link: https://lore.kernel.org/all/2a5ea27d-445e-4ffe-9447-9d7bfccd4a36@mail.kernel.org/
Signed-off-by: Mirza Ishan Beg <seedandsyntax@gmail.com>
---
drivers/media/dvb-core/dvb_frontend.c | 73 ++++++++++++++-------------
1 file changed, 38 insertions(+), 35 deletions(-)
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 0286da57f..f4b599a1a 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1342,13 +1342,19 @@ static int dtv_get_frontend(struct dvb_frontend *fe,
static int dvb_frontend_handle_ioctl(struct file *file,
unsigned int cmd, void *parg);
+enum dtv_get_shape {
+ SHAPE_DATA,
+ SHAPE_ST,
+ SHAPE_BUFFER,
+};
+
static int dtv_property_process_get(struct dvb_frontend *fe,
const struct dtv_frontend_properties *c,
struct dtv_property *tvp,
struct file *file)
{
+ enum dtv_get_shape shape = SHAPE_DATA;
int ncaps;
- unsigned int len = 1;
switch (tvp->cmd) {
case DTV_ENUM_DELSYS:
@@ -1358,7 +1364,7 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
ncaps++;
}
tvp->u.buffer.len = ncaps;
- len = ncaps;
+ shape = SHAPE_BUFFER;
break;
case DTV_FREQUENCY:
tvp->u.data = c->frequency;
@@ -1536,51 +1542,35 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
/* Fill quality measures */
case DTV_STAT_SIGNAL_STRENGTH:
tvp->u.st = c->strength;
- if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
- tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
- len = tvp->u.buffer.len;
+ shape = SHAPE_ST;
break;
case DTV_STAT_CNR:
tvp->u.st = c->cnr;
- if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
- tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
- len = tvp->u.buffer.len;
+ shape = SHAPE_ST;
break;
case DTV_STAT_PRE_ERROR_BIT_COUNT:
tvp->u.st = c->pre_bit_error;
- if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
- tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
- len = tvp->u.buffer.len;
+ shape = SHAPE_ST;
break;
case DTV_STAT_PRE_TOTAL_BIT_COUNT:
tvp->u.st = c->pre_bit_count;
- if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
- tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
- len = tvp->u.buffer.len;
+ shape = SHAPE_ST;
break;
case DTV_STAT_POST_ERROR_BIT_COUNT:
tvp->u.st = c->post_bit_error;
- if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
- tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
- len = tvp->u.buffer.len;
+ shape = SHAPE_ST;
break;
case DTV_STAT_POST_TOTAL_BIT_COUNT:
tvp->u.st = c->post_bit_count;
- if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
- tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
- len = tvp->u.buffer.len;
+ shape = SHAPE_ST;
break;
case DTV_STAT_ERROR_BLOCK_COUNT:
tvp->u.st = c->block_error;
- if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
- tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
- len = tvp->u.buffer.len;
+ shape = SHAPE_ST;
break;
case DTV_STAT_TOTAL_BLOCK_COUNT:
tvp->u.st = c->block_count;
- if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
- tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
- len = tvp->u.buffer.len;
+ shape = SHAPE_ST;
break;
default:
dev_dbg(fe->dvb->device,
@@ -1588,15 +1578,28 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
__func__, tvp->cmd);
return -EINVAL;
}
-
- if (len < 1)
- len = 1;
-
- dev_dbg(fe->dvb->device,
- "%s: GET cmd 0x%08x (%s) len %d: %*ph\n",
- __func__, tvp->cmd, dtv_cmd_name(tvp->cmd),
- tvp->u.buffer.len, tvp->u.buffer.len, tvp->u.buffer.data);
-
+ switch (shape) {
+ case SHAPE_BUFFER:
+ dev_dbg(fe->dvb->device,
+ "%s: GET cmd 0x%08x (%s) len %d: %*ph\n",
+ __func__, tvp->cmd, dtv_cmd_name(tvp->cmd),
+ tvp->u.buffer.len, tvp->u.buffer.len,
+ tvp->u.buffer.data);
+ break;
+ case SHAPE_ST:
+ dev_dbg(fe->dvb->device,
+ "%s: GET cmd 0x%08x (%s) len %d: %*ph\n",
+ __func__, tvp->cmd, dtv_cmd_name(tvp->cmd),
+ tvp->u.st.len, (int)sizeof(tvp->u.st),
+ &tvp->u.st);
+ break;
+ case SHAPE_DATA:
+ dev_dbg(fe->dvb->device,
+ "%s: GET cmd 0x%08x (%s) = 0x%08x\n",
+ __func__, tvp->cmd, dtv_cmd_name(tvp->cmd),
+ tvp->u.data);
+ break;
+ }
return 0;
}
--
2.56.0
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2026-10-09 14:23 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-09 14:23 [PATCH] media: dvb-core: fix OOB read in dtv_property_process_get() Mirza Ishan Beg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®