Re: [PATCH V2] thunderbolt: fix bandwidth group reservation indexing

[Mika Westerberg <mika.westerberg@linux.intel.com>]

Hi,

On Tue, Jun 23, 2026 at 09:37:59PM +0800, raoxu wrote:
> From: Xu Rao <raoxu@uniontech.com>
> 
> Group ID 0 is reserved, while valid bandwidth groups use IDs 1 through
> 7. MAX_GROUPS is used both for tb_cm::groups, which stores allocatable
> bandwidth groups, and for group_reserved[], which is indexed directly
> by Group ID.
> 
> tb_init_bandwidth_groups() assigns i + 1 to each entry in
> tb_cm::groups. Keeping seven entries therefore creates exactly the valid
> Group IDs 1 through 7.
> 
> However, group_reserved[MAX_GROUPS] currently also has seven entries,
> providing indices 0 through 6. When a tunnel belongs to Group ID 7,
> tb_consumed_dp_bandwidth() reads and may write one element past the end
> of the array. The reservation for that group is consequently not
> included in the consumed bandwidth total either.
> 
> Define MAX_GROUPS as 7 + 1 so arrays indexed directly by Group ID cover
> the complete 0 through 7 range, including the reserved ID 0. Size
> tb_cm::groups as MAX_GROUPS - 1 so only seven bandwidth group objects
> are initialized and tb_init_bandwidth_groups() continues to assign IDs
> 1 through 7. tb_consumed_dp_bandwidth() can then retain the direct
> Group ID indexing, with Group ID 7 selecting the final valid array
> element instead of accessing beyond it.
> 
> Fixes: 52a4490e89d7 ("thunderbolt: Reserve released DisplayPort bandwidth for a group for 10 seconds")
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
> ---
> Changes in v2:
> - Drop the zero-based Group ID conversion used in v1 and keep
>   group->index as the direct group_reserved[] index.
> - Include the reserved Group ID 0 in MAX_GROUPS so direct-indexed arrays
>   cover the complete Group ID range 0 through 7.
> - Size tb_cm::groups as MAX_GROUPS - 1 so
>   tb_init_bandwidth_groups() continues to create only the seven valid
>   groups with IDs 1 through 7.
> 
>  drivers/thunderbolt/tb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index 76323255439a..51f909db9383 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -41,7 +41,7 @@
>   */
>  #define TB_ASYM_THRESHOLD	45000
> 
> -#define MAX_GROUPS		7	/* max Group_ID is 7 */
> +#define MAX_GROUPS		(7 + 1) /* Group ID 0 is reserved */
> 
>  static unsigned int asym_threshold = TB_ASYM_THRESHOLD;
>  module_param_named(asym_threshold, asym_threshold, uint, 0444);
> @@ -66,7 +66,7 @@ struct tb_cm {
>  	struct list_head dp_resources;
>  	bool hotplug_active;
>  	struct delayed_work remove_work;
> -	struct tb_bandwidth_group groups[MAX_GROUPS];
> +	struct tb_bandwidth_group groups[MAX_GROUPS - 1];

Why this? We still need to be able to put there GroupIDs 1 to 7 (and keep
the 0 as is).

>  };
> 
>  static inline struct tb *tcm_to_tb(struct tb_cm *tcm)
> --
> 2.50.1