[PATCH v1] usb: typec: tcpm: Fix out-of-bounds read of the pds and pd_list arrays

[PATCH v1] usb: typec: tcpm: Fix out-of-bounds read of the pds and pd_list arrays

[Kyle Tso]

select_usb_power_delivery_show() iterates through port->pds (via the
pd_get() callback) and tcpm_find_pd_data() iterates through
port->pd_list until they find a NULL entry. However, tcpm_fw_get_caps()
allocates only port->pd_count entries for each array, and
tcpm_port_register_pd() fills all of them. Without a NULL terminator,
both loops read and dereference one entry past the end:

  Unable to handle kernel paging request at virtual address
  adacafaea9a8ac1a
  Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
  pc : select_usb_power_delivery_show+0xa4/0x110
  lr : select_usb_power_delivery_show+0x90/0x110
  Call trace:
   select_usb_power_delivery_show+0xa4/0x110
   dev_attr_show+0x38/0x74
   sysfs_kf_seq_show+0xb4/0x130
   kernfs_seq_show+0x44/0x54
   seq_read_iter+0x158/0x4ec
   kernfs_fop_read_iter+0x68/0x1bc
   vfs_read+0x230/0x2c0
   ksys_read+0x78/0xe8
   __arm64_sys_read+0x1c/0x2c
   invoke_syscall+0x58/0x118
   el0_svc_common+0xb4/0xf4
   do_el0_svc+0x24/0x80
   el0_svc+0x2c/0x90
   el0t_64_sync_handler+0x68/0xb4
   el0t_64_sync+0x1a4/0x1a8

Allocate one extra, zeroed entry for both arrays so that they are
always NULL-terminated.

Fixes: cd099cde4ed2 ("usb: typec: tcpm: Support multiple capabilities")
Cc: stable@vger.kernel.org
Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
Signed-off-by: Kyle Tso <kyletso@google.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index d721baef5210..8f4e10ec6d66 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -8376,14 +8376,21 @@ static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode
 		}
 	}
 
-	port->pds = devm_kcalloc(port->dev, port->pd_count, sizeof(struct usb_power_delivery *),
+	/*
+	 * Allocate one extra entry for each array to keep them NULL-terminated.
+	 * select_usb_power_delivery_show() iterates through port->pds (via the
+	 * pd_get callback) until it finds a NULL entry. Similarly,
+	 * tcpm_find_pd_data() iterates through port->pd_list until it finds a
+	 * NULL entry.
+	 */
+	port->pds = devm_kcalloc(port->dev, port->pd_count + 1, sizeof(struct usb_power_delivery *),
 				 GFP_KERNEL);
 	if (!port->pds) {
 		ret = -ENOMEM;
 		goto put_capabilities;
 	}
 
-	port->pd_list = devm_kcalloc(port->dev, port->pd_count, sizeof(struct pd_data *),
+	port->pd_list = devm_kcalloc(port->dev, port->pd_count + 1, sizeof(struct pd_data *),
 				     GFP_KERNEL);
 	if (!port->pd_list) {
 		ret = -ENOMEM;
-- 
2.56.0.385.gd3acb90ef8-goog

Re: [PATCH v1] usb: typec: tcpm: Fix out-of-bounds read of the pds and pd_list arrays

[sashiko-bot]

> select_usb_power_delivery_show() iterates through port->pds (via the
> pd_get() callback) and tcpm_find_pd_data() iterates through
> port->pd_list until they find a NULL entry. However, tcpm_fw_get_caps()
> allocates only port->pd_count entries for each array, and
> tcpm_port_register_pd() fills all of them. Without a NULL terminator,
> both loops read and dereference one entry past the end:
> 
>   Unable to handle kernel paging request at virtual address
>   adacafaea9a8ac1a
>   Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
>   pc : select_usb_power_delivery_show+0xa4/0x110
>   lr : select_usb_power_delivery_show+0x90/0x110
>   Call trace:
>    select_usb_power_delivery_show+0xa4/0x110
>    dev_attr_show+0x38/0x74
> [ ... ]
> 
> Fixes: cd099cde4ed2 ("usb: typec: tcpm: Support multiple capabilities")
> Cc: stable@vger.kernel.org
> Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
> Signed-off-by: Kyle Tso <kyletso@google.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013081549.2375441-1-kyletso@google.com?part=1