diff mbox series

thunderbolt: Fix KASAN reported stack out-of-bounds read in tb_retimer_scan()

Message ID 20241009060701.2434897-1-mika.westerberg@linux.intel.com
State New
Headers show
Series thunderbolt: Fix KASAN reported stack out-of-bounds read in tb_retimer_scan() | expand

Commit Message

Mika Westerberg Oct. 9, 2024, 6:07 a.m. UTC
KASAN reported following issue:

 BUG: KASAN: stack-out-of-bounds in tb_retimer_scan+0xffe/0x1550 [thunderbolt]
 Read of size 4 at addr ffff88810111fc1c by task kworker/u56:0/11
 CPU: 0 UID: 0 PID: 11 Comm: kworker/u56:0 Tainted: G     U             6.11.0+ #1387
 Tainted: [U]=USER
 Workqueue: thunderbolt0 tb_handle_hotplug [thunderbolt]
 Call Trace:
  <TASK>
  dump_stack_lvl+0x6c/0x90
  print_report+0xd1/0x630
  kasan_report+0xdb/0x110
  __asan_report_load4_noabort+0x14/0x20
  tb_retimer_scan+0xffe/0x1550 [thunderbolt]
  tb_scan_port+0xa6f/0x2060 [thunderbolt]
  tb_handle_hotplug+0x17b1/0x3080 [thunderbolt]
  process_one_work+0x626/0x1100
  worker_thread+0x6c8/0xfa0
  kthread+0x2c8/0x3a0
  ret_from_fork+0x3a/0x80
  ret_from_fork_asm+0x1a/0x30

This happens because the loop variable still gets incremented by one so
max becomes 3 instead of 2, and this makes the second loop read past the
the array declared on the stack.

Fix this by assigning to max directly in the loop body.

Fixes: ff6ab055e070 ("thunderbolt: Add receiver lane margining support for retimers")
CC: stable@vger.kernel.org
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/retimer.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Mika Westerberg Oct. 11, 2024, 11:24 a.m. UTC | #1
On Wed, Oct 09, 2024 at 09:07:01AM +0300, Mika Westerberg wrote:
> KASAN reported following issue:
> 
>  BUG: KASAN: stack-out-of-bounds in tb_retimer_scan+0xffe/0x1550 [thunderbolt]
>  Read of size 4 at addr ffff88810111fc1c by task kworker/u56:0/11
>  CPU: 0 UID: 0 PID: 11 Comm: kworker/u56:0 Tainted: G     U             6.11.0+ #1387
>  Tainted: [U]=USER
>  Workqueue: thunderbolt0 tb_handle_hotplug [thunderbolt]
>  Call Trace:
>   <TASK>
>   dump_stack_lvl+0x6c/0x90
>   print_report+0xd1/0x630
>   kasan_report+0xdb/0x110
>   __asan_report_load4_noabort+0x14/0x20
>   tb_retimer_scan+0xffe/0x1550 [thunderbolt]
>   tb_scan_port+0xa6f/0x2060 [thunderbolt]
>   tb_handle_hotplug+0x17b1/0x3080 [thunderbolt]
>   process_one_work+0x626/0x1100
>   worker_thread+0x6c8/0xfa0
>   kthread+0x2c8/0x3a0
>   ret_from_fork+0x3a/0x80
>   ret_from_fork_asm+0x1a/0x30
> 
> This happens because the loop variable still gets incremented by one so
> max becomes 3 instead of 2, and this makes the second loop read past the
> the array declared on the stack.
> 
> Fix this by assigning to max directly in the loop body.
> 
> Fixes: ff6ab055e070 ("thunderbolt: Add receiver lane margining support for retimers")
> CC: stable@vger.kernel.org
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Applied to thunderbolt.git/fixes.
diff mbox series

Patch

diff --git a/drivers/thunderbolt/retimer.c b/drivers/thunderbolt/retimer.c
index 721319329afa..7db9869a9f3f 100644
--- a/drivers/thunderbolt/retimer.c
+++ b/drivers/thunderbolt/retimer.c
@@ -516,7 +516,7 @@  int tb_retimer_scan(struct tb_port *port, bool add)
 	 */
 	tb_retimer_set_inbound_sbtx(port);
 
-	for (i = 1; i <= TB_MAX_RETIMER_INDEX; i++) {
+	for (max = 1, i = 1; i <= TB_MAX_RETIMER_INDEX; i++) {
 		/*
 		 * Last retimer is true only for the last on-board
 		 * retimer (the one connected directly to the Type-C
@@ -527,9 +527,10 @@  int tb_retimer_scan(struct tb_port *port, bool add)
 			last_idx = i;
 		else if (ret < 0)
 			break;
+
+		max = i;
 	}
 
-	max = i;
 	ret = 0;
 
 	/* Add retimers if they do not exist already */