Re: [bug report] thunderbolt: Allow USB3 bandwidth to be lower than maximum supported

[Gil Fine <gil.fine@linux.intel.com>]

Thanks Dan for the report !
This is indeed a bug and will be fixed in the subsequent kernel.
Have a nice day,
Gil

On Wed, Apr 17, 2024 at 03:52:34PM +0300, Dan Carpenter wrote:
> Hello Gil Fine,
> 
> Commit 25d905d2b819 ("thunderbolt: Allow USB3 bandwidth to be lower
> than maximum supported") from Feb 12, 2024 (linux-next), leads to the
> following Smatch static checker warning:
> 
> 	drivers/thunderbolt/tunnel.c:2113 tb_tunnel_alloc_usb3()
> 	error: uninitialized symbol 'max_rate'.
> 
> drivers/thunderbolt/tunnel.c
>     2061 struct tb_tunnel *tb_tunnel_alloc_usb3(struct tb *tb, struct tb_port *up,
>     2062                                        struct tb_port *down, int max_up,
>     2063                                        int max_down)
>     2064 {
>     2065         struct tb_tunnel *tunnel;
>     2066         struct tb_path *path;
>     2067         int max_rate;
>     2068 
>     2069         if (!tb_route(down->sw) && (max_up > 0 || max_down > 0)) {
>                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> The patch adds this condition.  If both of these are <= 0 then
> max_rate is uninitialized.
> 
>     2070                 /*
>     2071                  * For USB3 isochronous transfers, we allow bandwidth which is
>     2072                  * not higher than 90% of maximum supported bandwidth by USB3
>     2073                  * adapters.
>     2074                  */
>     2075                 max_rate = tb_usb3_max_link_rate(down, up);
>     2076                 if (max_rate < 0)
>     2077                         return NULL;
>     2078 
>     2079                 max_rate = max_rate * 90 / 100;
>     2080                 tb_port_dbg(up, "maximum required bandwidth for USB3 tunnel %d Mb/s\n",
>     2081                             max_rate);
>     2082         }
>     2083 
>     2084         tunnel = tb_tunnel_alloc(tb, 2, TB_TUNNEL_USB3);
>     2085         if (!tunnel)
>     2086                 return NULL;
>     2087 
>     2088         tunnel->activate = tb_usb3_activate;
>     2089         tunnel->src_port = down;
>     2090         tunnel->dst_port = up;
>     2091         tunnel->max_up = max_up;
>     2092         tunnel->max_down = max_down;
>     2093 
>     2094         path = tb_path_alloc(tb, down, TB_USB3_HOPID, up, TB_USB3_HOPID, 0,
>     2095                              "USB3 Down");
>     2096         if (!path) {
>     2097                 tb_tunnel_free(tunnel);
>     2098                 return NULL;
>     2099         }
>     2100         tb_usb3_init_path(path);
>     2101         tunnel->paths[TB_USB3_PATH_DOWN] = path;
>     2102 
>     2103         path = tb_path_alloc(tb, up, TB_USB3_HOPID, down, TB_USB3_HOPID, 0,
>     2104                              "USB3 Up");
>     2105         if (!path) {
>     2106                 tb_tunnel_free(tunnel);
>     2107                 return NULL;
>     2108         }
>     2109         tb_usb3_init_path(path);
>     2110         tunnel->paths[TB_USB3_PATH_UP] = path;
>     2111 
>     2112         if (!tb_route(down->sw)) {
> --> 2113                 tunnel->allocated_up = min(max_rate, max_up);
>                                                     ^^^^^^^^
>     2114                 tunnel->allocated_down = min(max_rate, max_down);
>                                                       ^^^^^^^^
> Uninitialized.
> 
>     2115 
>     2116                 tunnel->init = tb_usb3_init;
>     2117                 tunnel->consumed_bandwidth = tb_usb3_consumed_bandwidth;
>     2118                 tunnel->release_unused_bandwidth =
>     2119                         tb_usb3_release_unused_bandwidth;
>     2120                 tunnel->reclaim_available_bandwidth =
>     2121                         tb_usb3_reclaim_available_bandwidth;
>     2122         }
>     2123 
>     2124         return tunnel;
>     2125 }
> 
> regards,
> dan carpenter

-- 
Thanks,
Gil