[PATCH v2] thunderbolt: validate DROM entry lengths

[PATCH v2] thunderbolt: validate DROM entry lengths

[Pengpeng Hou]

tb_drom_parse_entries() checks that a DROM entry does not run past the
end of the DROM, but it does not require the declared entry length to
cover the entry header itself. A one-byte generic string entry therefore
makes the payload length calculation underflow.

Require each entry to contain its two-byte header before dispatching to
the type-specific DROM entry parsers. Also require a USB4 product
descriptor entry to contain the vendor and product fields read by its
parser.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1: https://lore.kernel.org/all/20260706091532.77293-1-pengpeng@iscas.ac.cn/
- use the full author name in From and Signed-off-by as requested by
  Mika Westerberg
- validate the fields read from USB4 product descriptor entries
- rebase onto the current tree

 drivers/thunderbolt/eeprom.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
index 5681c17f82ec..87ae1b8d7c82 100644
--- a/drivers/thunderbolt/eeprom.c
+++ b/drivers/thunderbolt/eeprom.c
@@ -348,6 +348,12 @@ static int tb_drom_parse_entry_generic(struct tb_switch *sw,
 		const struct tb_drom_entry_desc *desc =
 			(const struct tb_drom_entry_desc *)entry;
 
+		/* Header, USB spec, vendor ID, and product ID. */
+		if (header->len < sizeof(*header) + 3 * sizeof(u16)) {
+			tb_sw_warn(sw, "USB4 product descriptor entry is too short\n");
+			return -EIO;
+		}
+
 		if (!sw->vendor && !sw->device) {
 			sw->vendor = desc->idVendor;
 			sw->device = desc->idProduct;
@@ -414,9 +420,16 @@ static int tb_drom_parse_entries(struct tb_switch *sw, size_t header_size)
 	int res;
 
 	while (pos < drom_size) {
-		struct tb_drom_entry_header *entry = (void *) (sw->drom + pos);
-		if (pos + 1 == drom_size || pos + entry->len > drom_size
-				|| !entry->len) {
+		struct tb_drom_entry_header *entry;
+
+		if (drom_size - pos < sizeof(*entry)) {
+			tb_sw_warn(sw, "DROM buffer overrun\n");
+			return -EIO;
+		}
+
+		entry = (void *)(sw->drom + pos);
+		if (entry->len < sizeof(*entry) ||
+		    entry->len > drom_size - pos) {
 			tb_sw_warn(sw, "DROM buffer overrun\n");
 			return -EIO;
 		}
-- 
2.50.1

Re: [PATCH v2] thunderbolt: validate DROM entry lengths

[Mika Westerberg]

Hi,

On Fri, Jul 31, 2026 at 10:20:54PM +0800, Pengpeng Hou wrote:
> tb_drom_parse_entries() checks that a DROM entry does not run past the
> end of the DROM, but it does not require the declared entry length to
> cover the entry header itself. A one-byte generic string entry therefore
> makes the payload length calculation underflow.
> 
> Require each entry to contain its two-byte header before dispatching to
> the type-specific DROM entry parsers. Also require a USB4 product
> descriptor entry to contain the vendor and product fields read by its
> parser.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> Changes since v1: https://lore.kernel.org/all/20260706091532.77293-1-pengpeng@iscas.ac.cn/
> - use the full author name in From and Signed-off-by as requested by
>   Mika Westerberg
> - validate the fields read from USB4 product descriptor entries
> - rebase onto the current tree
> 
>  drivers/thunderbolt/eeprom.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
> index 5681c17f82ec..87ae1b8d7c82 100644
> --- a/drivers/thunderbolt/eeprom.c
> +++ b/drivers/thunderbolt/eeprom.c
> @@ -348,6 +348,12 @@ static int tb_drom_parse_entry_generic(struct tb_switch *sw,
>  		const struct tb_drom_entry_desc *desc =
>  			(const struct tb_drom_entry_desc *)entry;
>  
> +		/* Header, USB spec, vendor ID, and product ID. */

Can't you just compare to the desc here since that's the structure we find
there in that entry?

> +		if (header->len < sizeof(*header) + 3 * sizeof(u16)) {
> +			tb_sw_warn(sw, "USB4 product descriptor entry is too short\n");
> +			return -EIO;
> +		}
> +
>  		if (!sw->vendor && !sw->device) {
>  			sw->vendor = desc->idVendor;
>  			sw->device = desc->idProduct;
> @@ -414,9 +420,16 @@ static int tb_drom_parse_entries(struct tb_switch *sw, size_t header_size)
>  	int res;
>  
>  	while (pos < drom_size) {
> -		struct tb_drom_entry_header *entry = (void *) (sw->drom + pos);
> -		if (pos + 1 == drom_size || pos + entry->len > drom_size
> -				|| !entry->len) {
> +		struct tb_drom_entry_header *entry;
> +
> +		if (drom_size - pos < sizeof(*entry)) {
> +			tb_sw_warn(sw, "DROM buffer overrun\n");
> +			return -EIO;
> +		}
> +
> +		entry = (void *)(sw->drom + pos);
> +		if (entry->len < sizeof(*entry) ||
> +		    entry->len > drom_size - pos) {
>  			tb_sw_warn(sw, "DROM buffer overrun\n");
>  			return -EIO;
>  		}
> -- 
> 2.50.1