* [PATCH] media: dvb-core: fix OOB read in dtv_property_process_get()
@ 2026-10-09 14:23 Mirza Ishan Beg
2026-10-12 7:30 ` Slawomir Stepien
0 siblings, 1 reply; 2+ messages 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 related [flat|nested] 2+ messages in thread
* Re: [PATCH] media: dvb-core: fix OOB read in dtv_property_process_get()
2026-10-09 14:23 [PATCH] media: dvb-core: fix OOB read in dtv_property_process_get() Mirza Ishan Beg
@ 2026-10-12 7:30 ` Slawomir Stepien
0 siblings, 0 replies; 2+ messages in thread
From: Slawomir Stepien @ 2026-10-12 7:30 UTC (permalink / raw)
To: Mirza Ishan Beg
Cc: linux-media, linux-kernel, Mauro Carvalho Chehab, Hans Verkuil,
Kees Cook, syzbot+6102f1c3a0ea8073fd64
Hi
On paź 09, 2026 19:53, Mirza Ishan Beg wrote:
> 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>
Tested-by: Slawomir Stepien <sst@poczta.fm>
Reviewed-by: Slawomir Stepien <sst@poczta.fm>
Thanks for adding me! I think this patch is better than the v2 generated by syzkaller.
> ---
> 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;
> }
>
--
Slawomir Stepien
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-12 7:30 UTC | newest]
Thread overview: 2+ messages (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
2026-10-12 7:30 ` Slawomir Stepien
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox