All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC] media: dvb_frontend: fix OOB read in dtv_property_process_get()
@ 2026-09-29 17:00 syzbot
  2026-10-05 10:02 ` Slawomir Stepien
  0 siblings, 1 reply; 2+ messages in thread
From: syzbot @ 2026-09-29 17:00 UTC (permalink / raw)
  To: syzkaller-upstream-moderation; +Cc: syzbot

In dtv_property_process_get(), properties requested via the FE_GET_PROPERTY
ioctl are processed and copied back to userspace. In struct dtv_property,
member u is a union containing __u32 data, struct dtv_fe_stats st, and
struct buffer (which has __u8 data[32] followed by __u32 len at offset 32).
Commit 60f0618d157b ("media: dvb-core: frontend: make GET/SET safer")
introduced an out-of-bounds read and a data corruption issue due to
mishandling this union.

For commands that only set tvp->u.data (such as DTV_FREQUENCY),
tvp->u.buffer.len is never initialized by the kernel and retains whatever
value userspace passed into the ioctl. At the end of
dtv_property_process_get(), dev_dbg() unconditionally formatted the
property using "%*ph", passing tvp->u.buffer.len as the field width and
tvp->u.buffer.data as the buffer pointer. When dynamic debug is enabled and
userspace supplies a large length value, vsprintf() emits a field width out
of range warning, and hex_string() clamps the length to 64 bytes. Since
struct dtv_property is 76 bytes long and u.buffer.data starts at offset 16,
reading 64 bytes reaches offset 80 (4 bytes past the end of the slab
object), triggering a KASAN slab-out-of-bounds read:

field width 1163001896 out of range
WARNING: lib/vsprintf.c:2654 at vsnprintf+0xb9c/0xd10 lib/vsprintf.c:2918
Call trace:
 set_field_width lib/vsprintf.c:2654 [inline]
 vsnprintf+0xb9c/0xd10 lib/vsprintf.c:2918
 pointer+0x5e8/0x6b4 lib/vsprintf.c:2577
 vsnprintf+0x618/0xd10 lib/vsprintf.c:2953
 dev_vprintk_emit+0x2ec/0x3b4 drivers/base/core.c:5000
 dev_printk_emit+0xe0/0x130 drivers/base/core.c:5011
 __dynamic_dev_dbg+0x1b4/0x254 lib/dynamic_debug.c:904
 dtv_property_process_get+0x26bc/0x2838
 drivers/media/dvb-core/dvb_frontend.c:1595
 dvb_get_property drivers/media/dvb-core/dvb_frontend.c:2397 [inline]
 dvb_frontend_handle_ioctl+0x1644/0x2424
 drivers/media/dvb-core/dvb_frontend.c:2478
 dvb_frontend_do_ioctl+0x124/0x278
 drivers/media/dvb-core/dvb_frontend.c:2101
 dvb_usercopy+0x2a4/0x4d4 drivers/media/dvb-core/dvbdev.c:996
 dvb_frontend_ioctl+0x68/0x8c drivers/media/dvb-core/dvb_frontend.c:2115
BUG: KASAN: slab-out-of-bounds in hex_string+0x274/0x364
lib/vsprintf.c:1209
Read of size 1 at addr ffff0000c240314c

Additionally, commit 60f0618d157b added bounds checks that clamped
tvp->u.buffer.len in all eight DTV_STAT_* cases. However, DTV_STAT_*
commands populate tvp->u.st (struct dtv_fe_stats). In union u, u.buffer.len
is located at offset 32, which directly overlaps bytes 3 to 6 of
tvp->u.st.stat[3].uvalue (layer C statistics). Clamping and overwriting
tvp->u.buffer.len therefore silently corrupted the statistics returned to
userspace.

Fix this by only formatting with "%*ph" using tvp->u.buffer.len and
tvp->u.buffer.data when tvp->cmd == DTV_ENUM_DELSYS, which is the only
property command that populates the buffer union member. For all other
commands, format tvp->u.data as "= 0x%08x". Furthermore, remove the bogus
bounds clamping of tvp->u.buffer.len and the unused len variable from all
DTV_STAT_* cases to prevent corruption of frontend statistics.

Fixes: 60f0618d157b ("media: dvb-core: frontend: make GET/SET safer")

Fixes: 60f0618d157b ("media: dvb-core: frontend: make GET/SET safer")
Assisted-by: Gemini:gemini-3.8-flash Gemini:gemini-3.1-pro-preview syzbot
Reported-by: syzbot+6102f1c3a0ea8073fd64@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6102f1c3a0ea8073fd64
Link: https://syzkaller.appspot.com/ai_job?id=8f3957bb-b063-4d03-9e7e-dca8802ba8a9
To: <linux-media@vger.kernel.org>
To: "Mauro Carvalho Chehab" <mchehab@kernel.org>
Cc: "Hans Verkuil" <hverkuil+cisco@kernel.org>
Cc: "Kees Cook" <kees@kernel.org>
Cc: <linux-kernel@vger.kernel.org>
Cc: "Ricardo Ribalda" <ribalda@chromium.org>

---
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 0286da57f..e57f1935c 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1348,7 +1348,6 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 				    struct file *file)
 {
 	int ncaps;
-	unsigned int len = 1;
 
 	switch (tvp->cmd) {
 	case DTV_ENUM_DELSYS:
@@ -1358,7 +1357,6 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 			ncaps++;
 		}
 		tvp->u.buffer.len = ncaps;
-		len = ncaps;
 		break;
 	case DTV_FREQUENCY:
 		tvp->u.data = c->frequency;
@@ -1536,51 +1534,27 @@ 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;
 		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;
 		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;
 		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;
 		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;
 		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;
 		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;
 		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;
 		break;
 	default:
 		dev_dbg(fe->dvb->device,
@@ -1589,13 +1563,18 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 		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);
+	if (tvp->cmd == DTV_ENUM_DELSYS) {
+		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);
+	} else {
+		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);
+	}
 
 	return 0;
 }


base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
-- 
This is an AI-generated patch subject to moderation.
Reply with '#syz upstream' to Sign-off the patch as a human author
and send it to the upstream kernel mailing lists.
Reply with '#syz reject' to reject it ('#syz unreject' to undo).

See https://goo.gle/syzbot-ai-patches for information about AI-generated patches.
You can comment on the patch as usual, syzbot will try to address
the comments and send a new version of the patch if necessary.
syzbot engineers can be reached at syzkaller@googlegroups.com.

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH RFC] media: dvb_frontend: fix OOB read in dtv_property_process_get()
  2026-09-29 17:00 [PATCH RFC] media: dvb_frontend: fix OOB read in dtv_property_process_get() syzbot
@ 2026-10-05 10:02 ` Slawomir Stepien
  0 siblings, 0 replies; 2+ messages in thread
From: Slawomir Stepien @ 2026-10-05 10:02 UTC (permalink / raw)
  To: syzbot; +Cc: syzkaller-upstream-moderation, syzbot

On wrz 29, 2026 17:00, 'syzbot' via syzkaller-upstream-moderation wrote:
> In dtv_property_process_get(), properties requested via the FE_GET_PROPERTY
> ioctl are processed and copied back to userspace. In struct dtv_property,
> member u is a union containing __u32 data, struct dtv_fe_stats st, and
> struct buffer (which has __u8 data[32] followed by __u32 len at offset 32).
> Commit 60f0618d157b ("media: dvb-core: frontend: make GET/SET safer")
> introduced an out-of-bounds read and a data corruption issue due to
> mishandling this union.
> 
> For commands that only set tvp->u.data (such as DTV_FREQUENCY),
> tvp->u.buffer.len is never initialized by the kernel and retains whatever
> value userspace passed into the ioctl. At the end of
> dtv_property_process_get(), dev_dbg() unconditionally formatted the
> property using "%*ph", passing tvp->u.buffer.len as the field width and
> tvp->u.buffer.data as the buffer pointer. When dynamic debug is enabled and
> userspace supplies a large length value, vsprintf() emits a field width out
> of range warning, and hex_string() clamps the length to 64 bytes. Since
> struct dtv_property is 76 bytes long and u.buffer.data starts at offset 16,
> reading 64 bytes reaches offset 80 (4 bytes past the end of the slab
> object), triggering a KASAN slab-out-of-bounds read:
> 
> field width 1163001896 out of range
> WARNING: lib/vsprintf.c:2654 at vsnprintf+0xb9c/0xd10 lib/vsprintf.c:2918
> Call trace:
>  set_field_width lib/vsprintf.c:2654 [inline]
>  vsnprintf+0xb9c/0xd10 lib/vsprintf.c:2918
>  pointer+0x5e8/0x6b4 lib/vsprintf.c:2577
>  vsnprintf+0x618/0xd10 lib/vsprintf.c:2953
>  dev_vprintk_emit+0x2ec/0x3b4 drivers/base/core.c:5000
>  dev_printk_emit+0xe0/0x130 drivers/base/core.c:5011
>  __dynamic_dev_dbg+0x1b4/0x254 lib/dynamic_debug.c:904
>  dtv_property_process_get+0x26bc/0x2838
>  drivers/media/dvb-core/dvb_frontend.c:1595
>  dvb_get_property drivers/media/dvb-core/dvb_frontend.c:2397 [inline]
>  dvb_frontend_handle_ioctl+0x1644/0x2424
>  drivers/media/dvb-core/dvb_frontend.c:2478
>  dvb_frontend_do_ioctl+0x124/0x278
>  drivers/media/dvb-core/dvb_frontend.c:2101
>  dvb_usercopy+0x2a4/0x4d4 drivers/media/dvb-core/dvbdev.c:996
>  dvb_frontend_ioctl+0x68/0x8c drivers/media/dvb-core/dvb_frontend.c:2115
> BUG: KASAN: slab-out-of-bounds in hex_string+0x274/0x364
> lib/vsprintf.c:1209
> Read of size 1 at addr ffff0000c240314c
> 
> Additionally, commit 60f0618d157b added bounds checks that clamped
> tvp->u.buffer.len in all eight DTV_STAT_* cases. However, DTV_STAT_*
> commands populate tvp->u.st (struct dtv_fe_stats). In union u, u.buffer.len
> is located at offset 32, which directly overlaps bytes 3 to 6 of
> tvp->u.st.stat[3].uvalue (layer C statistics). Clamping and overwriting
> tvp->u.buffer.len therefore silently corrupted the statistics returned to
> userspace.
> 
> Fix this by only formatting with "%*ph" using tvp->u.buffer.len and
> tvp->u.buffer.data when tvp->cmd == DTV_ENUM_DELSYS, which is the only
> property command that populates the buffer union member. For all other
> commands, format tvp->u.data as "= 0x%08x". Furthermore, remove the bogus
> bounds clamping of tvp->u.buffer.len and the unused len variable from all
> DTV_STAT_* cases to prevent corruption of frontend statistics.
> 
> Fixes: 60f0618d157b ("media: dvb-core: frontend: make GET/SET safer")
> 
> Fixes: 60f0618d157b ("media: dvb-core: frontend: make GET/SET safer")
> Assisted-by: Gemini:gemini-3.8-flash Gemini:gemini-3.1-pro-preview syzbot
> Reported-by: syzbot+6102f1c3a0ea8073fd64@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=6102f1c3a0ea8073fd64
> Link: https://syzkaller.appspot.com/ai_job?id=8f3957bb-b063-4d03-9e7e-dca8802ba8a9
> To: <linux-media@vger.kernel.org>
> To: "Mauro Carvalho Chehab" <mchehab@kernel.org>
> Cc: "Hans Verkuil" <hverkuil+cisco@kernel.org>
> Cc: "Kees Cook" <kees@kernel.org>
> Cc: <linux-kernel@vger.kernel.org>
> Cc: "Ricardo Ribalda" <ribalda@chromium.org>
> 
> ---
> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
> index 0286da57f..e57f1935c 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -1348,7 +1348,6 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
>  				    struct file *file)
>  {
>  	int ncaps;
> -	unsigned int len = 1;
>  
>  	switch (tvp->cmd) {
>  	case DTV_ENUM_DELSYS:
> @@ -1358,7 +1357,6 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
>  			ncaps++;
>  		}
>  		tvp->u.buffer.len = ncaps;
> -		len = ncaps;
>  		break;
>  	case DTV_FREQUENCY:
>  		tvp->u.data = c->frequency;
> @@ -1536,51 +1534,27 @@ 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;
>  		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;
>  		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;
>  		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;
>  		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;
>  		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;
>  		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;
>  		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;
>  		break;
>  	default:
>  		dev_dbg(fe->dvb->device,
> @@ -1589,13 +1563,18 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
>  		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);
> +	if (tvp->cmd == DTV_ENUM_DELSYS) {
> +		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);
> +	} else {
> +		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);

But now, this dev_dbg will be used for DTV_STAT_* and the amount of tvp->u.data will not represent
the whole tvp->u.st. So here, not all bytes would be printed from stats struct. I think this is a
regression. Could you do better for DTV_STAT_*?

> +	}
>  
>  	return 0;
>  }
> 
> 
> base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e

-- 
Slawomir Stepien

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-05 10:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-29 17:00 [PATCH RFC] media: dvb_frontend: fix OOB read in dtv_property_process_get() syzbot
2026-10-05 10:02 ` Slawomir Stepien

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.