[bug report] drm/amdkfd: Fix the warning of array-index-out-of-bounds

[Ma, Jun]

Hi Dan,

Thanks for catching this. I'll check it and post a fix patch.

Regards,
Ma Jun

On 11/15/2022 9:00 PM, Dan Carpenter wrote:
> [ Ugh...  My email messed up and I have to Resend all my emails for the
>   past two weeks. -dan ]
> 
> Hello Ma Jun,
> 
> The patch c0cc999f3c32: "drm/amdkfd: Fix the warning of
> array-index-out-of-bounds" from Nov 2, 2022, leads to the following
> Smatch static checker warning:
> 
> 	drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:2008 kfd_topology_add_device()
> 	warn: inconsistent returns '&topology_lock'.
> 
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c
>     1808 int kfd_topology_add_device(struct kfd_dev *gpu)
>     1809 {
>     1810         uint32_t gpu_id;
>     1811         struct kfd_topology_device *dev;
>     1812         struct kfd_cu_info cu_info;
>     1813         int res = 0;
>     1814         struct list_head temp_topology_device_list;
>     1815         void *crat_image = NULL;
>     1816         size_t image_size = 0;
>     1817         int proximity_domain;
>     1818         int i;
>     1819         const char *asic_name = amdgpu_asic_name[gpu->adev->asic_type];
>     1820 
>     1821         INIT_LIST_HEAD(&temp_topology_device_list);
>     1822 
>     1823         gpu_id = kfd_generate_gpu_id(gpu);
>     1824         pr_debug("Adding new GPU (ID: 0x%x) to topology\n", gpu_id);
>     1825 
>     1826         /* Check to see if this gpu device exists in the topology_device_list.
>     1827          * If so, assign the gpu to that device,
>     1828          * else create a Virtual CRAT for this gpu device and then parse that
>     1829          * CRAT to create a new topology device. Once created assign the gpu to
>     1830          * that topology device
>     1831          */
>     1832         down_write(&topology_lock);
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^
> Takes the lock.
> 
>     1833         dev = kfd_assign_gpu(gpu);
>     1834         if (!dev) {
>     1835                 proximity_domain = ++topology_crat_proximity_domain;
>     1836 
>     1837                 res = kfd_create_crat_image_virtual(&crat_image, &image_size,
>     1838                                                     COMPUTE_UNIT_GPU, gpu,
>     1839                                                     proximity_domain);
>     1840                 if (res) {
>     1841                         pr_err("Error creating VCRAT for GPU (ID: 0x%x)\n",
>     1842                                gpu_id);
>     1843                         topology_crat_proximity_domain--;
>     1844                         return res;
> 
> Does not drop the lock.
> 
>     1845                 }
>     1846 
>     1847                 res = kfd_parse_crat_table(crat_image,
>     1848                                            &temp_topology_device_list,
>     1849                                            proximity_domain);
>     1850                 if (res) {
>     1851                         pr_err("Error parsing VCRAT for GPU (ID: 0x%x)\n",
>     1852                                gpu_id);
>     1853                         topology_crat_proximity_domain--;
>     1854                         goto err;
> 
> Does not drop the lock.
> 
>     1855                 }
>     1856 
>     1857                 kfd_topology_update_device_list(&temp_topology_device_list,
>     1858                         &topology_device_list);
>     1859 
>     1860                 dev = kfd_assign_gpu(gpu);
>     1861                 if (WARN_ON(!dev)) {
>     1862                         res = -ENODEV;
>     1863                         goto err;
> 
> Does not drop the lock.
> 
>     1864                 }
>     1865 
>     1866                 /* Fill the cache affinity information here for the GPUs
>     1867                  * using VCRAT
>     1868                  */
>     1869                 kfd_fill_cache_non_crat_info(dev, gpu);
>     1870 
>     1871                 /* Update the SYSFS tree, since we added another topology
>     1872                  * device
>     1873                  */
>     1874                 res = kfd_topology_update_sysfs();
>     1875                 if (!res)
>     1876                         sys_props.generation_count++;
>     1877                 else
>     1878                         pr_err("Failed to update GPU (ID: 0x%x) to sysfs topology. res=%d\n",
>     1879                                                 gpu_id, res);
>     1880         }
>     1881         up_write(&topology_lock);
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^
> Drops the lock.  The patch has some other locking changes which are
> not explained in the commit message and seem unrelated to the out of
> bounds issue.
> 
>     1882 
>     1883         dev->gpu_id = gpu_id;
>     1884         gpu->id = gpu_id;
> 
> regards,
> dan carpenter
>